Closed Bug 1343489 Opened 7 years ago Closed 7 years ago

Remove unnecessary IsNeckoChild check

Categories

(Core :: Networking: HTTP, enhancement)

enhancement
Not set
normal

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)

|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
Hi, I'm new to bugzilla. Mind if I take this one?
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 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+
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"?
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!
(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.
(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..)
Fixing commit message to fit the standard
Attachment #8842448 - Attachment is obsolete: true
Attachment #8842448 - Flags: review?(kechang)
Attachment #8842511 - Flags: review+
Thanks Patrick. I've uploaded a fixed patch. Is there anything else I need to do?
(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
Attachment #8842472 - Attachment is obsolete: true
Attachment #8842472 - Flags: review?(kechang)
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-
Keywords: checkin-needed
Assignee: nobody → nick
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dd9e290afb6
Remove unnecessary IsNeckoChild() call. r=mcmanus
Keywords: checkin-needed
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.