Closed Bug 1172011 Opened 6 years ago Closed 6 years ago
Splitter Frame .cpp: Value stored to 'space Left' is never read
Detected by scan-build: https://people.mozilla.org/~sledru/reports/fx-scan-build/report-nsSplitterFrame.cpp-ResizeChildTo-13-1.html#EndPath here https://dxr.mozilla.org/mozilla-central/source/layout/xul/nsSplitterFrame.cpp?from=nsSplitterFrame.cpp&case=true#1051 we should probably remove the declaration, it is useless and potentially missleading.
Hi Sylvestre, I've removed the declaration in this patch.
Riley, this bug is already assigned. You should find a bug without an assigned dev.
Sorry, it just seemed trivial and I wasn't sure the assignment to a Mozilla VP was legitimate. Moving on.
It was ;) Even for a VP, it is interesting to understand what is the contribution workflow. Anyway, I have many other bugs like that. Drop me an email if you are interested.
Actually, David has another bug assigned and I can give him more if he wants. Riley, about your patch, you should now find a reviewer (trick: look at the hg history of the file)
Assignee: dbryant → rileybaldinmozilla
Thanks Sylvestre, I'll shoot you an email later today. Arnaud, I'm adding you in because you recently made changes in this file. The author of this actual line was the creator of the file and I'm not sure how to find that (new to Mercurial). I'm requesting a review of the above patch.
Hi, Well "recently" isn't quite fair has I changed a few lines in this file more than one year ago :) I'm not allowed to review your patch sorry ;) You should look at the 'r=' part at the end of the commit message to find reviewers involved in this file As this file is in the layout part, you might need to find a layout module peer reviewer or a XUL peer reviewer. you can find module owners and peers here: https://wiki.mozilla.org/Modules/Core Don't be afraid to post thoses kind of questions on IRC irc.mozilla.org in #developers Thanks,
Fair enough. "The only 2+ line change in a year" may have been a better statement. Mike, I'm adding you in as you reviewed the last change to this file. If you're also unable to review I'll ping the IRC chat. I'm new to the workflow here. Thanks all.
I merely reviewed file moves.
Nathan, is it possible for you to review this patch?
Comment on attachment 8624537 [details] [diff] [review] removingDeclaration_diff.txt I'm not a layout/XUL peer, but I feel pretty confident in saying that this patch is OK after reviewing the scan-build output. For future reference, asking people to review patches is better done by setting the "review" flag on the patch directly: https://bugzilla.mozilla.org/attachment.cgi?id=8624537&action=edit on the above page, for instance, which can be reached by clicking the "details" link associated with your patch. You can set the review flag to "?", which will open up a textbox where you can enter the reviewer; it will also provide you with a list of suggested reviewers, which tends to be fairly accurate. I've set the review flag on this patch to "+" for you. Thanks for the patch!
Attachment #8624537 - Flags: review+
Requesting check-in. I don't have try server access.
Attachment #8625101 - Flags: checkin?(sledru)
Attachment #8624537 - Attachment is obsolete: true
Comment on attachment 8625101 [details] [diff] [review] Approved patch in hg format. Thanks for the patch, Riley :). In the future, please use the checkin-needed bug keyword instead. It plays more nicely with our bug marking tools :)
Attachment #8625101 - Flags: checkin?(sledru) → checkin+
You need to log in before you can comment on or make changes to this bug.