Last Comment Bug 448168 - Apply |s/entires/entries/g|
: Apply |s/entires/entries/g|
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: mozilla8
Assigned To: Robert Sesek
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on: 448110
Blocks:
  Show dependency treegraph
 
Reported: 2008-07-26 23:15 PDT by Serge Gautherie (:sgautherie)
Modified: 2011-08-05 08:48 PDT (History)
4 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (10.73 KB, patch)
2009-01-02 12:27 PST, Robert Sesek
no flags Details | Diff | Review
Patch v2 (11.74 KB, patch)
2009-01-04 11:49 PST, Robert Sesek
dveditz: review+
Details | Diff | Review
Patch v3 (9.47 KB, patch)
2011-07-03 03:40 PDT, Ed Morley [:emorley]
roc: review+
Details | Diff | Review
Patch v3.1 (9.46 KB, patch)
2011-08-01 16:34 PDT, Ed Morley [:emorley]
emorley: review+
Details | Diff | Review

Description Serge Gautherie (:sgautherie) 2008-07-26 23:15:27 PDT
 
Comment 1 Robert Sesek 2009-01-02 12:27:54 PST
Created attachment 355148 [details] [diff] [review]
Patch

This patch only touches comments, not any code.
Comment 2 Serge Gautherie (:sgautherie) 2009-01-02 21:51:47 PST
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.
Comment 3 Robert Sesek 2009-01-03 15:50:34 PST
(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.
Comment 4 Serge Gautherie (:sgautherie) 2009-01-04 08:33:58 PST
(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 5 Robert Sesek 2009-01-04 11:49:58 PST
Created attachment 355314 [details] [diff] [review]
Patch v2
Comment 6 Daniel Veditz [:dveditz] 2009-02-23 15:10:54 PST
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.
Comment 7 Serge Gautherie (:sgautherie) 2011-04-26 09:30:46 PDT
Robert, ping for progress status.
Comment 8 Ed Morley [:emorley] 2011-07-03 03:40:07 PDT
Created attachment 543664 [details] [diff] [review]
Patch v3

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.
Comment 9 Ed Morley [:emorley] 2011-07-03 03:44:36 PDT
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.
Comment 10 Ed Morley [:emorley] 2011-07-16 02:43:58 PDT
Ping for review (comment only changes). Thanks :-)
Comment 11 Ed Morley [:emorley] 2011-08-01 08:04:32 PDT
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 :-)
Comment 12 Ed Morley [:emorley] 2011-08-01 16:34:15 PDT
Created attachment 549967 [details] [diff] [review]
Patch v3.1

Updated commit message for r=roc, no other changes; carrying forwards r+.
Comment 13 Dão Gottwald [:dao] 2011-08-02 02:13:02 PDT
(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.
Comment 14 Ed Morley [:emorley] 2011-08-02 02:37:56 PDT
Roc reviewed the newer version, see comment 11.
Point taken about clarity, the feedback is appreciated :-)
Comment 15 Marco Bonardo [::mak] 2011-08-05 08:48:59 PDT
http://hg.mozilla.org/mozilla-central/rev/49f8fb1d048f

Note You need to log in before you can comment on or make changes to this bug.