Closed Bug 448168 Opened 13 years ago Closed 10 years ago
This patch only touches comments, not any code.
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
(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)...
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.
Whiteboard: [good first bug] → [patchlove] [good first bug]
Target Milestone: mozilla1.9.1a2 → ---
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.
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]
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?(roc) → review+
Updated commit message for r=roc, no other changes; carrying forwards r+.
(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 :-)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.