Bug 92500 – Activate committer bit granted back in April so that I can garden

• Component: New Bugs
• Status: RESOLVED FIXED

[Tom Hudson]

Activate committer bit granted back in April so that I can garden

[Tom Hudson]

Created attachment 154922 [details]
Patch

[WebKit Review Bot]

Attachment 154922 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1
Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Peter Beverloo]

Comment on attachment 154922 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154922&action=review

An easy way to create patches like this one is to make the code modification, and then use "webkit-patch upload". That takes care of all the required ChangeLog messages for you.

> Tools/ChangeLog:3
> +        Activate committer bit granted April 6th so I can garden.

Since r? is set, the "Reviewed by (OOPS!)" should be in the ChangeLog.
If you'd like to land this unreviewed, you may want to add a brief note mentioning why it's ok to land it unreviewed, i.e. "Unreviewed, moving myself in committers.py".

> Tools/Scripts/webkitpy/common/config/committers.py:373
> +    Committer("Tom Hudson", "[e-mail]"),

nit: You seem to be around on IRC as "tomhudson", it's convenient for notices (i.e. on reverts) by sheriffbot. Also, do you have an @chromium.org address?

[Tom Hudson]

Created attachment 154926 [details]
Patch

[Tom Hudson]

(In reply to comment #3)
> (From update of attachment 154922 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154922&action=review
> 
> An easy way to create patches like this one is to make the code modification, and then use "webkit-patch upload". That takes care of all the required ChangeLog messages for you.

I actually did use webkit-patch upload; I'm not sure what I did wrong with it.

> > Tools/Scripts/webkitpy/common/config/committers.py:373
> > +    Committer("Tom Hudson", "[e-mail]"),
> 
> nit: You seem to be around on IRC as "tomhudson", it's convenient for notices (i.e. on reverts) by sheriffbot. Also, do you have an @chromium.org address?

Thanks - I think I was discovering those myself at the same time you were posting here.

[Peter Beverloo]

Comment on attachment 154926 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154926&action=review

Almost there!

> Tools/ChangeLog:5
> +

nit: add the "Reviewed by NOBODY (OOPS!)." line. jochen__ on IRC will be happy to review this :).

> Tools/Scripts/webkitpy/common/config/committers.py:373
> +    Committer("Tom Hudson", "[e-mail]", "[e-mail]", "tomhudson"),

The e-mail addresses should be a list, i.e.

Committer("Tom Hudson", ["[e-mail]", "[e-mail]"], "tomhudson"),

[Tom Hudson]

Created attachment 154927 [details]
Patch

Comment on attachment 154927 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154927&action=review

> Tools/ChangeLog:3
> +        Activate committer bit granted April 6th so I can garden.

nit "Update my email address and irc nick" would be a better changelog description, no?

[Tom Hudson]

(In reply to comment #8)
> (From update of attachment 154927 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154927&action=review
> 
> > Tools/ChangeLog:3
> > +        Activate committer bit granted April 6th so I can garden.
> 
> nit "Update my email address and irc nick" would be a better changelog description, no?

I thought the significant fact that needed to go in the changelog was moving me from Contributors: to Committers:

[WebKit Review Bot]

Comment on attachment 154927 [details]
Patch

Rejecting attachment 154927 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[Peter Beverloo]

Comment on attachment 154927 [details]
Patch

Setting cq+.

[WebKit Review Bot]

Comment on attachment 154927 [details]
Patch

Rejecting attachment 154927 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
410-ab3c-d52691b4dbfc ...
Currently at 123873 = ac0cb6ade7cad7a46d3800ba5b8d9fff931c2986
r123874 = c63ad0593b841b826df1d10fe8242e3c73e492c2
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
RA layer request failed: Server sent unexpected return value (503 Service Temporarily Unavailable) in response to OPTIONS request for 'http://svn.webkit.org/repository/webkit' at /usr/lib/git-core/git-svn line 2290

Died at Tools/Scripts/update-webkit line 164.

Full output: http://queues.webkit.org/results/13375402

[WebKit Review Bot]

Comment on attachment 154927 [details]
Patch

Rejecting attachment 154927 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[WebKit Review Bot]

Comment on attachment 154927 [details]
Patch

Clearing flags on attachment: 154927

Committed r123892: <http://trac.webkit.org/changeset/123892>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

One is supposed to land changes to this file manually, not have someone else cq+ it. That's how you prove that you actually have committer privileges.

[Peter Beverloo]

(In reply to comment #16)
> One is supposed to land changes to this file manually, not have someone else cq+ it. That's how you prove that you actually have committer privileges.

I was never told anything like this when I became a committer, and judging by a quick peek by similar changes made by other recent new committers, neither were they.

If such a policy exist, then it should be communicated much more clearly.

[Peter Beverloo]

To be clear, I of course don't doubt existence of such a policy if you say it does. We can't expect new contributors to immediately understand all our (sometimes unwritten) policies, though, and this seems like a good candidate to clarify on, for example in the initial e-mail sent by Brian.

I have modified the to-do item on the wiki page for now.
https://trac.webkit.org/wiki/CommitterTips