Closed Bug 448168 Opened 11 years ago Closed 8 years ago

Apply |s/entires/entries/g|

Categories

(Core :: General, defect, trivial)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: sgautherie, Assigned: rsesek)

References

()

Details

(Whiteboard: [inbound])

Attachments

(1 file, 3 obsolete files)

 
Whiteboard: [good first bug]
Attached patch Patch (obsolete) — Splinter Review
This patch only touches comments, not any code.
Attachment #355148 - Flags: review?
Comment on attachment 355148 [details] [diff] [review]
Patch

This patch is missing
/security/nss/lib/ssl/ssl.h

You need to choose a reviewer:
either a "global" one, or it may be easier to separate the patch per code areas.
Assignee: nobody → rsesek
Status: NEW → ASSIGNED
Attachment #355148 - Attachment is obsolete: true
Attachment #355148 - Flags: review?
(In reply to comment #2)
> (From update of attachment 355148 [details] [diff] [review])
> This patch is missing
> /security/nss/lib/ssl/ssl.h
> 
> You need to choose a reviewer:
> either a "global" one, or it may be easier to separate the patch per code
> areas.

Thanks. I've fixed that one. Where can I find a global reviewer (don't see them listed in the owners list)? If I were to separate this out into the individual modules, that'd be 8 different patches, which seems a little excessive for such a simple change.
(In reply to comment #3)

> Where can I find a global reviewer (don't see them listed in the owners list)?

Sure, the closest are super-reviewers...

> If I were to separate this out into the individual modules, that'd be 8
> different patches, which seems a little excessive for such a simple change.

No trivial answer: see what is easier to get review(s)...
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #355314 - Flags: review?(dveditz)
Attachment #355314 - Attachment is patch: true
Attachment #355314 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 355314 [details] [diff] [review]
Patch v2

r/sr=dveditz for everything except the two security/nss files. You'll have to get the NSS team to review those.
Attachment #355314 - Flags: review?(dveditz) → review+
Robert, ping for progress status.
Flags: in-testsuite-
Whiteboard: [good first bug] → [patchlove] [good first bug]
Target Milestone: mozilla1.9.1a2 → ---
No longer depends on: 448156
Attached patch Patch v3 (obsolete) — Splinter Review
Updated patch on behalf of Robert, so we can get this into the tree. Some of the instances of "entires" no longer exist, but new ones have cropped up. Patch format corrected + author set to Robert.

Re-requesting review due to changes now being made to files not included in the Robert's previous patch.
Attachment #355314 - Attachment is obsolete: true
Attachment #543664 - Flags: review?(dveditz)
Meant to say that there is now only one instance of "entires" in NSS (http://mxr.mozilla.org/mozilla-central/source/security/nss/lib/ssl/ssl.h#386), so have just missed out NSS changes for now.
Whiteboard: [patchlove] [good first bug] → [good first bug]
Assignee: rsesek → bmo
Whiteboard: [good first bug]
Ping for review (comment only changes). Thanks :-)
Comment on attachment 543664 [details] [diff] [review]
Patch v3

Adding alternative reviewer for this comment-only typo fix patch, so we can get a new contributor's first patch into the tree. Thanks :-)
Attachment #543664 - Flags: review?(roc)
Attachment #543664 - Flags: review?(dveditz)
Attached patch Patch v3.1Splinter Review
Updated commit message for r=roc, no other changes; carrying forwards r+.
Attachment #543664 - Attachment is obsolete: true
Attachment #549967 - Flags: review+
Assignee: bmo → rsesek
Keywords: checkin-needed
(In reply to comment #12)
> carrying forwards r+.

This isn't helpful, just adds confusion. You didn't review the patch, dveditz did -- and the patch incorrectly says roc.
Roc reviewed the newer version, see comment 11.
Point taken about clarity, the feedback is appreciated :-)
Keywords: checkin-needed
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/49f8fb1d048f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.