Remove unnecessary IsNeckoChild check

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kershaw, Assigned: njfox)

Tracking

({good-first-bug})

Trunk
mozilla54
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: [necko-backlog])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

2 years ago
|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

2 years ago
Hi, I'm new to bugzilla. Mind if I take this one?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

2 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

2 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

2 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

2 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!
(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..)
(Assignee)

Comment 10

2 years ago
Fixing commit message to fit the standard
Attachment #8842448 - Attachment is obsolete: true
Attachment #8842448 - Flags: review?(kechang)
(Assignee)

Updated

2 years ago
Attachment #8842511 - Flags: review+
(Assignee)

Comment 11

2 years ago
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-
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Assignee: nobody → nick

Comment 15

2 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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0dd9e290afb6
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.