Closed Bug 1337303 Opened 6 years ago Closed 6 years ago

Fix the wrong comment in nsHttpChunkedDecoder


(Core :: Networking, defect)

Not set



Tracking Status
firefox54 --- fixed


(Reporter: kershaw, Assigned: njfox)


(Keywords: good-first-bug, Whiteboard: [necko-backlog])


(1 file, 2 obsolete files)

Keywords: good-first-bug
Whiteboard: [necko-backlog]
Submitted changeset for review.
Reviewed by mcmanus; I had attached this to the wrong bug before.
Attachment #8842509 - Flags: review+
Attachment #8842509 - Flags: checkin+
Attachment #8842509 - Flags: checkin+
Comment on attachment 8842509 [details] [diff] [review]
Bug 1337303 - Corrected RFC number cited in comments

Review of attachment 8842509 [details] [diff] [review]:

somehow you have created a file called commit-message-f58fb you don't want a new file in the repo like that.
Attachment #8842509 - Flags: review+ → review-
Comment on attachment 8842574 [details]
Bug 1337303 - Corrected RFC number cited in comments

duplicate review request
Attachment #8842574 - Flags: review?(mcmanus)
Attachment #8842509 - Attachment is obsolete: true
Attachment #8842574 - Attachment is obsolete: true
Attachment #8842622 - Flags: review+
Keywords: checkin-needed
Latest attachment should be good to go. This is a comment-only change, so this patch should not need a try run to prove it's ok. Thanks for your patience while I learned the workflow for contributing.
Pushed by
Corrected RFC number cited in comments. r=mcmanus
Keywords: checkin-needed
Assignee: nobody → nick
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
nick, thanks for the patch and this all worked out ok so we don't need to take any action.

But in comment 4 I revoked your r+ and changed it to a r-.. that meant you didn't have a r+ to self-assign and then mark checkin-needed when you fixed the patch. (you fixed it correctly, so I would have then given you the r+.. but I changed it to r- because I wanted to confirm it as you're a new contributor.. I should have made that more clear to you when I did it - its certainly not common.) I'm sure it was just done out of confusion (afterall I shouldn't have r+'d the patch at all if I was unhappy - I just realized afterwards what I was seeing in there with the extra file).

no worries - just clarifying the workflow because the tools (intentionally) let people override this stuff.
Thanks Patrick, and sorry about that. It is definitely out of confusion :) I am reaching out to people on IRC to figure out what I'm doing wrong.
You need to log in before you can comment on or make changes to this bug.