Changeset 85065 in webkit


Ignore:
Timestamp:
Apr 27, 2011 11:16:50 AM (13 years ago)
Author:
eric@webkit.org
Message:

2011-04-27 Eric Seidel <​eric@webkit.org>

Reviewed by Mihai Parparita.

sherrifbot create-bug shouldn't assign bugs to webkit.review.bot
​https://bugs.webkit.org/show_bug.cgi?id=59545

To do this, I needed a way to look up contributors by irc-name
(since anyone in #webkit who might use this command may not be a committer).
To lookup contributors, I had to make Contributor a real object.
Which led me to redesign parts of committers.py...
and finally fix one spot in changelog.py where we wanted to be
looking up contributors and not committers.

Overall a pretty simple fix, once you wade through the yak-hair.

This may not prevent *all* possible ways that bugs would get assigned
to webkit.review.bot. If we don't recognize the requester we will
go through the previous code path (which shouldn't change the assignee
on the bug from the default as far as I can tell).

  • Scripts/webkitpy/common/checkout/changelog.py:
  • Scripts/webkitpy/common/config/committers.py:
  • Scripts/webkitpy/common/config/committers_unittest.py:
  • Scripts/webkitpy/tool/bot/irc_command.py:
  • Scripts/webkitpy/tool/bot/irc_command_unittest.py:
Location:
trunk/Tools
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r85062 r85065  
     12011-04-27  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Mihai Parparita.
     4
     5        sherrifbot create-bug shouldn't assign bugs to webkit.review.bot
     6        https://bugs.webkit.org/show_bug.cgi?id=59545
     7
     8        To do this, I needed a way to look up contributors by irc-name
     9        (since anyone in #webkit who might use this command may not be a committer).
     10        To lookup contributors, I had to make Contributor a real object.
     11        Which led me to redesign parts of committers.py...
     12        and finally fix one spot in changelog.py where we wanted to be
     13        looking up contributors and not committers.
     14
     15        Overall a pretty simple fix, once you wade through the yak-hair.
     16
     17        This may not prevent *all* possible ways that bugs would get assigned
     18        to webkit.review.bot.  If we don't recognize the requester we will
     19        go through the previous code path (which shouldn't change the assignee
     20        on the bug from the default as far as I can tell).
     21
     22        * Scripts/webkitpy/common/checkout/changelog.py:
     23        * Scripts/webkitpy/common/config/committers.py:
     24        * Scripts/webkitpy/common/config/committers_unittest.py:
     25        * Scripts/webkitpy/tool/bot/irc_command.py:
     26        * Scripts/webkitpy/tool/bot/irc_command_unittest.py:
     27
    1282011-04-27  Yi Shen  <yi.4.shen@nokia.com>
    229

  • trunk/Tools/Scripts/webkitpy/common/checkout/changelog.py

    r82196 r85065  
    6565
    6666        self._reviewer = self._committer_list.committer_by_name(self._reviewer_text)
    67         self._author = self._committer_list.committer_by_email(self._author_email) or self._committer_list.committer_by_name(self._author_name)
     67        self._author = self._committer_list.contributor_by_email(self._author_email) or self._committer_list.contributor_by_name(self._author_name)
    6868
    6969    def author_name(self):
    … …  
    7474
    7575    def author(self):
    76         return self._author # Might be None
     76        return self._author  # Might be None
    7777
    7878    # FIXME: Eventually we would like to map reviwer names to reviewer objects.
    … …  
    8282
    8383    def reviewer(self):
    84         return self._reviewer # Might be None
     84        return self._reviewer  # Might be None, might also not be a Reviewer!
    8585
    8686    def contents(self):

  • trunk/Tools/Scripts/webkitpy/common/config/committers.py

    r85049 r85065  
    11# Copyright (c) 2011, Apple Inc. All rights reserved.
    2 # Copyright (c) 2009, Google Inc. All rights reserved.
     2# Copyright (c) 2009, 2011 Google Inc. All rights reserved.
    33#
    44# Redistribution and use in source and binary forms, with or without
    … …  
    2828# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    2929#
    30 # WebKit's Python module for committer and reviewer validation
    31 
    32 
    33 class Contributor:
    34 
     30# WebKit's Python module for committer and reviewer validation.
     31
     32
     33class Contributor(object):
    3534    def __init__(self, name, email_or_emails, irc_nickname=None):
    36         return
    37 
    38 class Committer:
    39 
    40     def __init__(self, name, email_or_emails, irc_nickname=None):
     35        assert(name)
     36        assert(email_or_emails)
    4137        self.full_name = name
    4238        if isinstance(email_or_emails, str):
    … …  
    4541            self.emails = email_or_emails
    4642        self.irc_nickname = irc_nickname
     43        self.can_commit = False
    4744        self.can_review = False
    4845
    … …  
    5653
    5754
     55class Committer(Contributor):
     56    def __init__(self, name, email_or_emails, irc_nickname=None):
     57        Contributor.__init__(self, name, email_or_emails, irc_nickname)
     58        self.can_commit = True
     59
     60
    5861class Reviewer(Committer):
    59 
    6062    def __init__(self, name, email_or_emails, irc_nickname=None):
    6163        Committer.__init__(self, name, email_or_emails, irc_nickname)
    … …  
    347349
    348350
    349 class CommitterList:
     351class CommitterList(object):
    350352
    351353    # Committers and reviewers are passed in to allow easy testing
    352 
    353354    def __init__(self,
    354355                 committers=committers_unable_to_review,
    355                  reviewers=reviewers_list):
     356                 reviewers=reviewers_list,
     357                 contributors=contributors_who_are_not_committers):
     358        self._contributors = contributors + committers + reviewers
    356359        self._committers = committers + reviewers
    357360        self._reviewers = reviewers
    358         self._committers_by_email = {}
     361        self._contributors_by_email = {}
     362
     363    def contributors(self):
     364        return self._contributors
    359365
    360366    def committers(self):
    … …  
    364370        return self._reviewers
    365371
    366     def _email_to_committer_map(self):
    367         if not len(self._committers_by_email):
    368             for committer in self._committers:
    369                 for email in committer.emails:
    370                     self._committers_by_email[email] = committer
    371         return self._committers_by_email
     372    def _email_to_contributor_map(self):
     373        if not len(self._contributors_by_email):
     374            for contributor in self._contributors:
     375                for email in contributor.emails:
     376                    assert(email not in self._contributors_by_email)  # We should never have duplicate emails.
     377                    self._contributors_by_email[email] = contributor
     378        return self._contributors_by_email
     379
     380    def _committer_only(self, record):
     381        if record and not record.can_commit:
     382            return None
     383        return record
     384
     385    def _reviewer_only(self, record):
     386        if record and not record.can_review:
     387            return None
     388        return record
     389
     390    def contributor_by_name(self, name):
     391        # This could be made into a hash lookup if callers need it to be fast.
     392        for contributor in self.contributors():
     393            if contributor.full_name and contributor.full_name == name:
     394                return contributor
     395        return None
    372396
    373397    def committer_by_name(self, name):
    374         # This could be made into a hash lookup if callers need it to be fast.
    375         for committer in self.committers():
    376             if committer.full_name == name:
    377                 return committer
     398        return self._committer_only(self.contributor_by_name(name))
     399
     400    def contributor_by_irc_nickname(self, irc_nickname):
     401        for contributor in self.contributors():
     402            if contributor.irc_nickname and contributor.irc_nickname == irc_nickname:
     403                return contributor
     404        return None
     405
     406    def contributor_by_email(self, email):
     407        return self._email_to_contributor_map().get(email)
    378408
    379409    def committer_by_email(self, email):
    380         return self._email_to_committer_map().get(email)
     410        return self._committer_only(self.contributor_by_email(email))
    381411
    382412    def reviewer_by_email(self, email):
    383         committer = self.committer_by_email(email)
    384         if committer and not committer.can_review:
    385             return None
    386         return committer
     413        return self._reviewer_only(self.contributor_by_email(email))

  • trunk/Tools/Scripts/webkitpy/common/config/committers_unittest.py

    r56504 r85065  
    2828
    2929import unittest
    30 from webkitpy.common.config.committers import CommitterList, Committer, Reviewer
     30from webkitpy.common.config.committers import CommitterList, Contributor, Committer, Reviewer
    3131
    3232class CommittersTest(unittest.TestCase):
    … …  
    3535        committer = Committer('Test One', 'one@test.com', 'one')
    3636        reviewer = Reviewer('Test Two', ['two@test.com', 'two@rad.com', 'so_two@gmail.com'])
    37         committer_list = CommitterList(committers=[committer], reviewers=[reviewer])
     37        contributor = Contributor('Test Three', ['three@test.com'], 'three')
     38        committer_list = CommitterList(committers=[committer], reviewers=[reviewer], contributors=[contributor])
    3839
    39         # Test valid committer and reviewer lookup
     40        # Test valid committer, reviewer and contributor lookup
    4041        self.assertEqual(committer_list.committer_by_email('one@test.com'), committer)
    4142        self.assertEqual(committer_list.reviewer_by_email('two@test.com'), reviewer)
    … …  
    4344        self.assertEqual(committer_list.committer_by_email('two@rad.com'), reviewer)
    4445        self.assertEqual(committer_list.reviewer_by_email('so_two@gmail.com'), reviewer)
     46        self.assertEqual(committer_list.contributor_by_email('three@test.com'), contributor)
    4547
    46         # Test valid committer and reviewer lookup
     48        # Test valid committer, reviewer and contributor lookup
    4749        self.assertEqual(committer_list.committer_by_name("Test One"), committer)
    4850        self.assertEqual(committer_list.committer_by_name("Test Two"), reviewer)
    4951        self.assertEqual(committer_list.committer_by_name("Test Three"), None)
     52        self.assertEqual(committer_list.contributor_by_name("Test Three"), contributor)
    5053
    5154        # Test that the first email is assumed to be the Bugzilla email address (for now)
    … …  
    5457        # Test that a known committer is not returned during reviewer lookup
    5558        self.assertEqual(committer_list.reviewer_by_email('one@test.com'), None)
     59        self.assertEqual(committer_list.reviewer_by_email('three@test.com'), None)
     60        # and likewise that a known contributor is not returned for committer lookup.
     61        self.assertEqual(committer_list.committer_by_email('three@test.com'), None)
    5662
    5763        # Test that unknown email address fail both committer and reviewer lookup
    … …  
    6369
    6470        self.assertEqual(committer.irc_nickname, 'one')
     71        self.assertEqual(committer_list.contributor_by_irc_nickname('one'), committer)
     72        self.assertEqual(committer_list.contributor_by_irc_nickname('three'), contributor)
    6573
    66         # Test that committers returns committers and reviewers and reviewers() just reviewers.
     74        # Test that the lists returned are are we expect them.
     75        self.assertEqual(committer_list.contributors(), [contributor, committer, reviewer])
    6776        self.assertEqual(committer_list.committers(), [committer, reviewer])
    6877        self.assertEqual(committer_list.reviewers(), [reviewer])
    69 
    70 
    71 if __name__ == '__main__':
    72     unittest.main()

  • trunk/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py

    r71856 r85065  
    2929# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    3030
     31from webkitpy.common.memoized import memoized
    3132from webkitpy.common.system.deprecated_logging import log
    3233
    … …  
    3940        self._attachment_dictionary = attachment_dictionary
    4041        self._bug = bug
     42        # FIXME: These should be replaced with @memoized after updating mocks.
    4143        self._reviewer = None
    4244        self._committer = None
    … …  
    4850        return int(self._attachment_dictionary.get("id"))
    4951
    50     def attacher_is_committer(self):
    51         return self._bugzilla.committers.committer_by_email(
    52             patch.attacher_email())
     52    @memoized
     53    def attacher(self):
     54        return self._bugzilla().committers.contributor_by_email(self.attacher_email())
    5355
    5456    def attacher_email(self):
    … …  
    9698        if not email:
    9799            return None
     100        # FIXME: This is not a robust way to call committer_by_email
    98101        committer = getattr(self._bugzilla().committers,
    99102                            "%s_by_email" % flag)(email)
    … …  
    104107                 self._attachment_dictionary['bug_id'], flag, email))
    105108
     109    # FIXME: These could use @memoized like attacher(), but unit tests would need updates.
    106110    def reviewer(self):
    107111        if not self._reviewer:

  • trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py

    r84955 r85065  
    116116            return "%s: Usage: BUGZILLA_EMAIL" % nick
    117117        email = args[0]
    118         committer = CommitterList().committer_by_email(email)
     118        # FIXME: We should get the ContributorList off the tool somewhere.
     119        committer = CommitterList().contributor_by_email(email)
    119120        if not committer:
    120121            return "%s: Sorry, I don't know %s. Maybe you could introduce me?" % (nick, email)
    … …  
    144145        bug_description = "%s\nRequested by %s on %s." % (bug_title, nick, config_irc.channel)
    145146
     147        # There happens to be a committers list hung off of Bugzilla, so
     148        # re-using that one makes things easiest for now.
     149        requester = tool.bugs.committers.contributor_by_irc_nickname(nick)
     150        requester_email = requester.bugzilla_email() if requester else None
     151
    146152        try:
    147             bug_id = tool.bugs.create_bug(bug_title, bug_description)
     153            bug_id = tool.bugs.create_bug(bug_title, bug_description, cc=requester_email, assignee=requester_email)
    148154            bug_url = tool.bugs.bug_url_for_bug_id(bug_id)
    149155            return "%s: Created bug: %s" % (nick, bug_url)

  • trunk/Tools/Scripts/webkitpy/tool/bot/irc_command_unittest.py

    r84955 r85065  
    5757                          create_bug.execute("tom", [], None, None))
    5858
     59        example_args = ["sherrif-bot", "should", "have", "a", "create-bug", "command"]
    5960        tool = MockTool()
     61
     62        # MockBugzilla has a create_bug, but it logs to stderr, this avoids any logging.
     63        tool.bugs.create_bug = lambda a, b, cc=None, assignee=None: 78
    6064        self.assertEquals("tom: Created bug: http://example.com/78",
    61                           create_bug.execute("tom", ["sherrif-bot", "should", "have", "a", "create-bug", "command"], tool, None))
     65                          create_bug.execute("tom", example_args, tool, None))
    6266
    63         def mock_create_bug(title, description):
     67        def mock_create_bug(title, description, cc=None, assignee=None):
    6468            raise Exception("Exception from bugzilla!")
    6569        tool.bugs.create_bug = mock_create_bug
    6670        self.assertEquals("tom: Failed to create bug:\nException from bugzilla!",
    67                           create_bug.execute("tom", ["sherrif-bot", "should", "have", "a", "create-bug", "command"], tool, None))
     71                          create_bug.execute("tom", example_args, tool, None))

  • trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py

    r76008 r85065  
    160160# patches to be uploaded by committers, who are generally trustworthy folk. :)
    161161class AbstractCommitterOnlyEWS(AbstractEarlyWarningSystem):
    162     def __init__(self, committers=CommitterList()):
    163         AbstractEarlyWarningSystem.__init__(self)
    164         self._committers = committers
    165 
    166162    def process_work_item(self, patch):
    167         if not self._committers.committer_by_email(patch.attacher_email()):
     163        if not patch.attacher() or not patch.attacher().can_commit:
    168164            self._did_error(patch, "%s cannot process patches from non-committers :(" % self.name)
    169165            return False

  • trunk/Tools/Scripts/webkitpy/tool/mocktool.py

    r84575 r85065  
    538538            "author_name": "Adam Barth",
    539539            "author_email": "abarth@webkit.org",
    540             "author": self._committer_list.committer_by_email("abarth@webkit.org"),
     540            "author": self._committer_list.contributor_by_email("abarth@webkit.org"),
    541541            "reviewer_text": "Darin Adler",
    542542            "reviewer": self._committer_list.committer_by_name("Darin Adler"),
Note: See TracChangeset for help on using the changeset viewer.