Fix the wrong comment in nsHttpChunkedDecoder

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kershaw, Assigned: njfox)

Tracking

({good-first-bug})

Trunk
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: [necko-backlog])

Attachments

(1 attachment, 2 obsolete attachments)

Keywords: good-first-bug
Whiteboard: [necko-backlog]
Assignee

Comment 1

2 years ago
Submitted changeset for review.
Assignee

Comment 2

2 years ago
Reviewed by mcmanus; I had attached this to the wrong bug before.
Attachment #8842509 - Flags: review+
Attachment #8842509 - Flags: checkin+
Assignee

Updated

2 years ago
Attachment #8842509 - Flags: checkin+
Comment hidden (mozreview-request)
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 5

2 years ago
mozreview-review
Comment on attachment 8842574 [details]
Bug 1337303 - Corrected RFC number cited in comments

https://reviewboard.mozilla.org/r/116358/#review118004

duplicate review request
Attachment #8842574 - Flags: review?(mcmanus)
Assignee

Comment 6

2 years ago
Attachment #8842509 - Attachment is obsolete: true
Attachment #8842574 - Attachment is obsolete: true
Attachment #8842622 - Flags: review+
Assignee

Updated

2 years ago
Keywords: checkin-needed
Assignee

Comment 7

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

Comment 8

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/74b41dce2486
Corrected RFC number cited in comments. r=mcmanus
Keywords: checkin-needed
Assignee: nobody → nick
Status: NEW → ASSIGNED

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/74b41dce2486
Status: ASSIGNED → RESOLVED
Last Resolved: 2 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.
Assignee

Comment 11

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