Closed
Bug 1343489
Opened 7 years ago
Closed 7 years ago
Remove unnecessary IsNeckoChild check
Categories
(Core :: Networking: HTTP, enhancement)
Core
Networking: HTTP
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: kershaw, Assigned: njfox)
Details
(Keywords: good-first-bug, Whiteboard: [necko-backlog])
Attachments
(1 file, 3 obsolete files)
1.39 KB,
patch
|
njfox
:
review+
|
Details | Diff | Splinter Review |
|IsNeckoChild()| is already called at the start of the function. We can remove the unnecessary one at [1]. [1] http://searchfox.org/mozilla-central/rev/9c1c7106eef137e3413fd867fc1ddfb1d3f6728c/netwerk/protocol/http/nsHttpHandler.cpp#2260
Assignee | ||
Comment 1•7 years ago
|
||
Hi, I'm new to bugzilla. Mind if I take this one?
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8842448 [details] Removed unnecessary IsNeckoChild() call per bug 1343489 https://reviewboard.mozilla.org/r/116294/#review117904 ::: commit-message-34c6c:1 (Diff revision 1) > +Removed unnecessary IsNeckoChild() call per bug 1343489 just make sure you change this to the traditional form.. i.e. Bug 1343489 - Remove unnecessary IsNeckoChild() call r=mcmanus also make sure your reviewers are module peers or owners thanks for the patch!
Attachment #8842448 -
Flags: review?(mcmanus) → review+
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8842472 [details] Corrected RFC number cited in comments. https://reviewboard.mozilla.org/r/116300/#review117906 I'm going to r+ this, but you need to open a different bug for it, upload it there and check it with that bug number. It has nothing to do with the bug you have attached it to. thanks for the patch.
Attachment #8842472 -
Flags: review?(mcmanus) → review+
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8842448 [details] Removed unnecessary IsNeckoChild() call per bug 1343489 https://reviewboard.mozilla.org/r/116294/#review117904 > just make sure you change this to the traditional form.. i.e. > > Bug 1343489 - Remove unnecessary IsNeckoChild() call r=mcmanus > > also make sure your reviewers are module peers or owners > > thanks for the patch! Sorry, this was my first time contributing! Thank you for the review. Just so I understand, are you saying going forward the commit message should be: "Bug 1343489 - Remove unnecessary IsNeckoChild() call r=mcmanus"?
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8842472 [details] Corrected RFC number cited in comments. https://reviewboard.mozilla.org/r/116300/#review117906 Sorry about that, as I mentioned in the other patch, I'm new to this :P. Would you mind giving me more detailed instructions for how to do this? I tried asking on IRC but I didn't get a response. Thanks!
Comment 8•7 years ago
|
||
(In reply to nick from comment #6) > so I understand, are you saying going forward the commit message should be: > "Bug 1343489 - Remove unnecessary IsNeckoChild() call r=mcmanus"? yes, that is the usual template. you can do hg log and see lots of examples. This is just for the first line of the commit message. If you would like to include more information you can do that freeform in the rest of the message - but the first line follows a strong template.
Comment 9•7 years ago
|
||
(In reply to nick from comment #7) > Comment on attachment 8842472 [details] > Corrected RFC number cited in comments. > > https://reviewboard.mozilla.org/r/116300/#review117906 > > Sorry about that, as I mentioned in the other patch, I'm new to this :P. > Would you mind giving me more detailed instructions for how to do this? I > tried asking on IRC but I didn't get a response. > > Thanks! just take your patch and upload it bugzilla 1337303 .. change the commit message to reflect that bug number and r=mcmanus. you can manually mark it r+ by clicking on the details screen once you have uploaded it to that bug. (Its an honor system - even though our rules don't allow you to review your own patch, you can mark a patch as r+ yourself for convenience if the review comes out of band which it sometimes does. (verbally over a shoulder, or an update with an pre-agreed upon change with the reviewer, etc..)
Assignee | ||
Comment 10•7 years ago
|
||
Fixing commit message to fit the standard
Attachment #8842448 -
Attachment is obsolete: true
Attachment #8842448 -
Flags: review?(kechang)
Assignee | ||
Updated•7 years ago
|
Attachment #8842511 -
Flags: review+
Assignee | ||
Comment 11•7 years ago
|
||
Thanks Patrick. I've uploaded a fixed patch. Is there anything else I need to do?
Comment 12•7 years ago
|
||
(In reply to nick from comment #11) > Thanks Patrick. I've uploaded a fixed patch. Is there anything else I need > to do? https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
Updated•7 years ago
|
Attachment #8842472 -
Attachment is obsolete: true
Attachment #8842472 -
Flags: review?(kechang)
Comment 13•7 years ago
|
||
Comment on attachment 8842511 [details] [diff] [review] Bug 1343489 - Remove unnecessary IsNeckoChild() call Review of attachment 8842511 [details] [diff] [review]: ----------------------------------------------------------------- I just realized this patch also contains a new file called commit-message-blah .. that's supposed to be in the commit message, not a new file in the tree. that needs to be fixed.
Attachment #8842511 -
Flags: review+ → review-
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8842511 -
Attachment is obsolete: true
Attachment #8842621 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Updated•7 years ago
|
Assignee: nobody → nick
Comment 15•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0dd9e290afb6 Remove unnecessary IsNeckoChild() call. r=mcmanus
Keywords: checkin-needed
Comment 16•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0dd9e290afb6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•