RESOLVED FIXED 71690
Add cc-bugs group to watch changes in chromium graphics
https://bugs.webkit.org/show_bug.cgi?id=71690
Summary Add cc-bugs group to watch changes in chromium graphics
Dana Jansens
Reported 2011-11-07 08:04:54 PST
Add cc-bugs group to watch changes in chromium graphics
Attachments
Patch (1.45 KB, patch)
2011-11-07 08:06 PST, Dana Jansens
no flags
add cc-bugs to a new committers.py list "watcher_accounts" (2.45 KB, patch)
2011-11-08 08:18 PST, Dana Jansens
no flags
create non-contributor account list and make check-webkit-style use the full account list (7.34 KB, patch)
2011-11-09 11:13 PST, Dana Jansens
no flags
Patch (11.69 KB, patch)
2011-11-09 16:04 PST, Dana Jansens
no flags
Dana Jansens
Comment 1 2011-11-07 08:06:06 PST
WebKit Review Bot
Comment 2 2011-11-07 08:06:58 PST
Attachment 113868 [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/Scripts/webkitpy/common/config/watchlist:0: The email alias cc-bugs@google.com which is in the watchlist is not listed as a contributor in committers.py [watchlist/general] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Adam Barth
Comment 3 2011-11-07 09:24:14 PST
Comment on attachment 113868 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113868&action=review > Tools/Scripts/webkitpy/common/config/watchlist:118 > + "ChromiumGraphics": [ "jamesr@chromium.org", "cc-bugs@google.com" ], Does cc-bugs have a bugs.webkit.org account?
Dana Jansens
Comment 4 2011-11-07 09:37:23 PST
Yes, made one for it.
WebKit Review Bot
Comment 5 2011-11-07 13:30:56 PST
Comment on attachment 113868 [details] Patch Clearing flags on attachment: 113868 Committed r99470: <http://trac.webkit.org/changeset/99470>
WebKit Review Bot
Comment 6 2011-11-07 13:31:00 PST
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 7 2011-11-07 15:31:54 PST
This broke test-webkitpy. :(
Eric Seidel (no email)
Comment 8 2011-11-07 15:33:32 PST
The test seems poorly designed: FAIL: test_watch_list_load (webkitpy.common.watchlist.watchlistloader_unittest.WatchListLoaderTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Projects/WebKit/Tools/Scripts/webkitpy/common/watchlist/watchlistloader_unittest.py", line 45, in test_watch_list_load OutputCapture().assert_outputs(self, WatchListLoader(filesystem.FileSystem()).load, expected_logs="") File "/Projects/WebKit/Tools/Scripts/webkitpy/common/system/outputcapture.py", line 79, in assert_outputs testcase.assertEqual(logs_string, expected_logs) AssertionError: 'The email alias cc-bugs@google.com which is in the watchlist is not listed as a contributor in committers.py\n' != '' Señor Levin?
David Levin
Comment 9 2011-11-07 16:13:04 PST
Rolled out here due to test breakage (which the style bot flagged): http://trac.webkit.org/changeset/99494 There are two options: 1. Add that to the contributors or 2. Add a new section to committers.py (known accounts) that I can query. The problem is that it is super bad if an email gets added to this file without having a bugzilla account and this is my best way of checking that. (fwiw, if they don't have a bugzilla account, then the bug update fails and no cc, message. etc. gets added.) #2 is a good option if you don't need autocomplete. I'm willing to advise if you need help. Thanks for understanding!
Eric Seidel (no email)
Comment 10 2011-11-07 16:19:33 PST
There must be a better way to solve this. Either by some sort of cc-attempt fallback code, or some way to query bugzilla about known accounts. We have a way today with webkit-patch find-users, but it requires you to have admin privileges on bugs.webkit.org
Dana Jansens
Comment 11 2011-11-08 08:16:22 PST
I will go with #2 and add a new list to committers.py, named watcher_accounts, for bugzilla accounts that watch bugs but don't contribute.
Dana Jansens
Comment 12 2011-11-08 08:18:11 PST
Created attachment 114077 [details] add cc-bugs to a new committers.py list "watcher_accounts"
Dana Jansens
Comment 13 2011-11-08 08:18:57 PST
This will still fail style/break the test until it grabs the watcher_accounts list and deals with it accordingly.
WebKit Review Bot
Comment 14 2011-11-08 08:25:20 PST
Attachment 114077 [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/Scripts/webkitpy/common/config/watchlist:0: The email alias cc-bugs@google.com which is in the watchlist is not listed as a contributor in committers.py [watchlist/general] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
David Levin
Comment 15 2011-11-08 09:32:50 PST
(In reply to comment #13) > This will still fail style/break the test until it grabs the watcher_accounts list and deals with it accordingly. I misspoke "Add a new section to committers.py (known accounts) that I can query." I meant "Add a new section to committers.py (known accounts) that the tool can query." but then rest shouldn't be hard. Hook up your new list so that it can be queried like contributor_by_email. Perhaps accounts_by_email? and change this line to call it: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py#L155 Then, everything will be fixed. If you need more verbose instructions, let me know and I can walk you through it.
Dana Jansens
Comment 16 2011-11-09 11:13:32 PST
Created attachment 114319 [details] create non-contributor account list and make check-webkit-style use the full account list
Dana Jansens
Comment 17 2011-11-09 11:16:15 PST
Thanks for the thoughtful instructions. I've followed through them and made the requested changes. Now you query the list of "accounts" instead of "contributors" in the style check.
Eric Seidel (no email)
Comment 18 2011-11-09 11:34:50 PST
Comment on attachment 114319 [details] create non-contributor account list and make check-webkit-style use the full account list View in context: https://bugs.webkit.org/attachment.cgi?id=114319&action=review > Tools/Scripts/webkitpy/common/config/committers.py:90 > +# This is a list of email addresses who have bugzilla accounts but are not A list is not a "who", is it? > Tools/Scripts/webkitpy/common/config/committers.py:95 > + Contributor("Chromium Compositor Bugs", ["cc-bugs@google.com"], "cc-bugs"), This should be Account(...) > Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:158 > - if not contributors.contributor_by_email(email): > + if not accounts.account_by_email(email): > cc_rule.remove_instruction(email) > self._log_error("The email alias %s which is in the watchlist is not listed as a contributor in committers.py" % email) > continue This code should additionally check that it's the first email, the bugzilla_email in the account. account_by_email will give you any account with that email, which might not be the bugzilla email for the account.
David Levin
Comment 19 2011-11-09 11:36:07 PST
(In reply to comment #18) > (From update of attachment 114319 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=114319&action=review > > > Tools/Scripts/webkitpy/common/config/committers.py:90 > > +# This is a list of email addresses who have bugzilla accounts but are not > > A list is not a "who", is it? > > > Tools/Scripts/webkitpy/common/config/committers.py:95 > > + Contributor("Chromium Compositor Bugs", ["cc-bugs@google.com"], "cc-bugs"), > > This should be Account(...) > > > Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:158 > > - if not contributors.contributor_by_email(email): > > + if not accounts.account_by_email(email): > > cc_rule.remove_instruction(email) > > self._log_error("The email alias %s which is in the watchlist is not listed as a contributor in committers.py" % email) > > continue > > This code should additionally check that it's the first email, the bugzilla_email in the account. account_by_email will give you any account with that email, which might not be the bugzilla email for the account. And please add a simple test to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers_unittest.py? Thanks so much for doing this!
Dana Jansens
Comment 20 2011-11-09 16:04:46 PST
Dana Jansens
Comment 21 2011-11-09 16:06:17 PST
Made an additional account_by_login() that returns an Account for a login email address. Added unit tests, and addressed nits.
WebKit Review Bot
Comment 22 2011-11-09 17:07:27 PST
Comment on attachment 114384 [details] Patch Clearing flags on attachment: 114384 Committed r99786: <http://trac.webkit.org/changeset/99786>
WebKit Review Bot
Comment 23 2011-11-09 17:07:33 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.