http://sylvestre.ledru.info/reports/fx-scan-build/report-MoofParser.cpp-Parse-5-1.html#EndPath Seems that the version variable is useless. Reported for a potential new contributor.

Hello, I would like to fix this if it's possible.

I had the impression that the version variable is not even used?

Comment on attachment 8966194 [details] [diff] [review] patch : unused variable 'version' deleted Looks good. Now, you should fix a reviewer for this: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_Get_your_code_reviewed

Comment on attachment 8966194 [details] [diff] [review] patch : unused variable 'version' deleted Actually, your patch is incorrect. it is built upon the previous one. The patch should be built from the up to date m-c tree.

I'm not sure to understand what you mean.. I tried to pull changes (hg pull), then I deleted the unused variable 'version' then I commited (hg commit -m "...") and then I created my patch (hg export . > my-change.patch) and this is the patch I asked for a review. Now when I'm trying to pull (hg pull) it says that I'm up to date, and when I'm looking into the file dom/media/mp4/MoofParser.cpp line 933 the variable 'version' isn't here anymore so I don't know if my patch has been reviewed or if it's because I have local commit ? (I tried to removed it with hg commit --amend but it doesn't do anything ..) Thanks for your help.

Your patch should contain: - uint8_t version = flags >> 24; Currently, you have two patches doing: - uint8_t version = flags >> 24; + uint8_t version; and - uint8_t version; is it more clear?

Ok yes it's what I understood before but my problem now is that I cann't find any way to have the previous version : I tried to delete the file and then do "hg update --clean" to have the current version of the file but in this new file I still don't have the variable as if I already deleted it. So my question is : can I have back the current version of the file (without my changes) without downloading everything again ? Like if I had done nothing yet. Because now I don't have anything to change in the file because the variable is not there anymore ..

sorry :) A few commands: "hg wip" will give you the tree state "hg histedit" (like git rebase -i) will give you the opportunity to fold the two patches into one. "hg strip -r <revision>" drop the change Hope this helps

Ok so now I can see that in the tree I made two patchs so I understand that I have to delete one or like merge them together but I'm trying to do that but I don't find how .. And I have an error when I try "hg histedit -o" (abandon : cannot edit history that contains merges) Any easy way (in my opinion) to do is to restart everything from zero, just like I never did anything, any patch, any changes, but I'm not able to do that, I tried to purge everything, but it's still doesn't work ..

you probably need to rebase: hg rebase -s <revision source aka your local changes> -d <the top of the official tree> you can get the revisions using "hg wip"

Ok I think I managed to correct the bug (just deleted the unused variable 'version') and made only one patch.

Comment on attachment 8966989 [details] [diff] [review] Bug 1445220 - I erased the initialisation of the variable 'version' and make a single patch for it Good, you should now look for a reviewer.

I tried to find a reviewer by looking who change the function where I made my change with "hg blame -u" but I cannot select people that I found because they have not logged in to Bugzilla within the last 60 days... And it's the same for all users I tried.. How can I do ? Can I find a reviewer in an other way ?

Comment on attachment 8966989 [details] [diff] [review] Bug 1445220 - I erased the initialisation of the variable 'version' and make a single patch for it Review of attachment 8966989 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, I only reviewed your code previously. But we want the whole patch to be ready to land in Firefox, and I think its commit description should be updated. Currently it only says what you've done in the code (good!), and something about patches which is not interesting to keep in the patch itself. I would suggest you change the commit description, by using e.g.: `hg commit --amend`. Ideally the description should tell us what you've done but also very importantly *why* you've done it. In this case, it could be as simple as "Removed unused variable" (what=removed var, why=unused) ;-) Please re-submit the patch with the new description and I'll be happy to give 'r+' again. In the meantime, the code looked fine so I've started the next step: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_6_Getting_code_into_the_tree I assume you don't have 'try' access, so I pushed your patch on your behalf; you can see its progress there: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4c21556b4874475d398142111e64648a665d0d7 I'm pretty sure this will succeed, so we're very close to getting your code into Firefox!

https://hg.mozilla.org/integration/mozilla-inbound/rev/19f68300761ed2d550be280215da2103fa39a629 Bug 1445220 - Removed 'version' variable declaration as it was unused r=gerald

I pushed it with a proper comment to close the loop here after 11 days on inactivity!

Thank you Sylvestre. Was I too harsh?

Not at all, this is just that it has been reported a month and half ago, I am going on pto for a week and was clearing my backlog :) merci!

Sorry, I didn't have access on my computer for severals days... Thank you Sylvestre.

No worries, bravo for your first patch, it is not in nightly

(In reply to Sylvestre Ledru [:sylvestre] [back May 7th] from comment #23) > No worries, bravo for your first patch, it is not in nightly I think you mean it is *now* in nightly. ;-) (I make the same typo all the time -- let's blame auto-correct) And welcome to the party, cubouyaka!