Attachment 115741 Details for Bug 72690 – fixes the bug

[patch] fixes the bug

bug-72690-20111117222444.patch (text/plain), 9.13 KB, created by Ryosuke Niwa on 2011-11-17 22:24:45 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2011-11-17 22:24:45 PST

Size: 9.13 KB

patch

obsolete

>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 100725)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,20 @@
>+2011-11-17  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        ChangeLogEntry should be able to parse entries with multiple authors
>+        https://bugs.webkit.org/show_bug.cgi?id=72690
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Support multi-author change log entries in ChangeLogEntry. Split author names using
>+        the same logic as splitting reviewer names. _authors now stores list of dictionaries
>+        with "name" and "email" as keys. Unlike reviewers, we can't always resolve authors to
>+        accounts because some contributors are never listed in committers.py. 
>+
>+        * Scripts/webkitpy/common/checkout/changelog.py:
>+        * Scripts/webkitpy/common/checkout/changelog_unittest.py:
>+        * Scripts/webkitpy/common/config/committers.py: Fixed a bug that *_by_{name|email}
>+        throws an exception when the argument is None.
>+
> 2011-11-17  Raphael Kubo da Costa  <kubo@profusion.mobi>
> 
>         [EFL] Clean up the use of DATA_DIR in the buildsystem
>Index: Tools/Scripts/webkitpy/common/checkout/changelog.py
>===================================================================
>--- Tools/Scripts/webkitpy/common/checkout/changelog.py	(revision 100708)
>+++ Tools/Scripts/webkitpy/common/checkout/changelog.py	(working copy)
>@@ -34,6 +34,7 @@ import re
> import textwrap
> 
> from webkitpy.common.config.committers import CommitterList
>+from webkitpy.common.config.committers import Account
> import webkitpy.common.config.urls as config_urls
> from webkitpy.common.system.deprecated_logging import log
> 
>@@ -71,7 +72,7 @@ def parse_bug_id_from_changelog(message)
> 
> class ChangeLogEntry(object):
>     # e.g. 2009-06-03  Eric Seidel  <eric@webkit.org>
>-    date_line_regexp = r'^(?P<date>\d{4}-\d{2}-\d{2})\s+(?P<name>.+?)\s+<(?P<email>[^<>]+)>$'
>+    date_line_regexp = r'^(?P<date>\d{4}-\d{2}-\d{2})\s+(?P<authors>(?P<name>[^<]+?)\s+<(?P<email>[^<>]+)>.*?)$'
> 
>     # e.g. * Source/WebCore/page/EventHandler.cpp: Implement FooBarQuux.
>     touched_files_regexp = r'^\s*\*\s*(?P<file>[A-Za-z0-9_\-\./\\]+)\s*\:'
>@@ -135,9 +136,7 @@ class ChangeLogEntry(object):
>         if not len(reviewer_text):
>             return None, None
> 
>-        # FIXME: Canonicalize reviewer names; e.g. Andy "First Time Reviewer" Estes
>-        # FIXME: Ignore NOBODY (\w+) and "a spell checker"
>-        reviewer_list = re.split(r'\s*(?:(?:,(?:\s+and\s+|&)?)|(?:and\s+|&)|(?:[/+]))\s*', reviewer_text)
>+        reviewer_list = ChangeLogEntry._split_contributor_names(reviewer_text)
> 
>         # Get rid of "reviewers" like "even though this is just a..." in "Reviewed by Sam Weinig, even though this is just a..."
>         # and "who wrote the original code" in "Noam Rosenthal, who wrote the original code"
>@@ -145,6 +144,10 @@ class ChangeLogEntry(object):
> 
>         return reviewer_text, reviewer_list
> 
>+    @staticmethod
>+    def _split_contributor_names(text):
>+        return re.split(r'\s*(?:,(?:\s+and\s+|&)?|(?:^|\s+)and\s+|[/+&])\s*', text)
>+
>     def _fuzz_match_reviewers(self, reviewers_text_list):
>         if not reviewers_text_list:
>             return []
>@@ -158,24 +161,36 @@ class ChangeLogEntry(object):
>             log("WARNING: Creating invalid ChangeLogEntry:\n%s" % self._contents)
> 
>         # FIXME: group("name") does not seem to be Unicode?  Probably due to self._contents not being unicode.
>-        self._author_name = match.group("name") if match else None
>-        self._author_email = match.group("email") if match else None
>+        self._author_text = match.group("authors") if match else None
>+        self._authors = []
>+        if self._author_text:
>+            authors = ChangeLogEntry._split_contributor_names(self._author_text)
>+            assert(authors and len(authors) >= 1)
>+            for author in authors:
>+                author_match = re.match(r'(?P<name>.+?)\s+<(?P<email>[^>]+)>', author)
>+                self._authors.append({'name': author_match.group("name"), 'email': author_match.group("email")})
> 
>         self._reviewer_text, self._reviewers_text_list = ChangeLogEntry._parse_reviewer_text(self._contents)
>         self._reviewers = self._fuzz_match_reviewers(self._reviewers_text_list)
>-        self._author = self._committer_list.contributor_by_email(self._author_email) or self._committer_list.contributor_by_name(self._author_name)
>+        self._author = self._committer_list.contributor_by_email(self.author_email()) or self._committer_list.contributor_by_name(self.author_name())
> 
>         self._touched_files = re.findall(self.touched_files_regexp, self._contents, re.MULTILINE)
> 
>+    def author_text(self):
>+        return self._author_text
>+
>     def author_name(self):
>-        return self._author_name
>+        return self._authors[0]['name']
> 
>     def author_email(self):
>-        return self._author_email
>+        return self._authors[0]['email']
> 
>     def author(self):
>         return self._author  # Might be None
> 
>+    def authors(self):
>+        return self._authors
>+
>     # FIXME: Eventually we would like to map reviwer names to reviewer objects.
>     # See https://bugs.webkit.org/show_bug.cgi?id=26533
>     def reviewer_text(self):
>Index: Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py
>===================================================================
>--- Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py	(revision 100708)
>+++ Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py	(working copy)
>@@ -322,6 +322,14 @@ class ChangeLogTest(unittest.TestCase):
>         self._assert_parse_reviewer_text_list('Reviewed by NOBODY (Build fix, forgot to svn add this file)', None)
>         self._assert_parse_reviewer_text_list('Reviewed by nobody (trivial follow up fix), Joseph Pecoraro LGTM-ed.', None)
> 
>+    def _entry_with_author(self, author_text):
>+        return ChangeLogEntry('''2009-08-19  AUTHOR_TEXT
>+
>+            Reviewed by Ryosuke Niwa
>+
>+            * Scripts/bugzilla-tool:
>+'''.replace("AUTHOR_TEXT", author_text))
>+
>     def _entry_with_reviewer(self, reviewer_line):
>         return ChangeLogEntry('''2009-08-19  Eric Seidel  <eric@webkit.org>
> 
>@@ -350,6 +358,19 @@ class ChangeLogTest(unittest.TestCase):
>         self._assert_fuzzy_reviewer_match('Reviewed by Darin Adler in <https://bugs.webkit.org/show_bug.cgi?id=47736>.', ['Darin Adler in'], ['Darin Adler'])
>         self._assert_fuzzy_reviewer_match('Reviewed by Adam Barth.:w', ['Adam Barth.:w'], ['Adam Barth'])
> 
>+    def _assert_parse_authors(self, author_text, expected_contributors):
>+        parsed_authors = [(author['name'], author['email']) for author in self._entry_with_author(author_text).authors()]
>+        self.assertEquals(parsed_authors, expected_contributors)
>+
>+    def test_parse_authors(self):
>+        self._assert_parse_authors(u'Aaron Colwell  <acolwell@chromium.org>', [(u'Aaron Colwell', u'acolwell@chromium.org')])
>+        self._assert_parse_authors('Eric Seidel  <eric@webkit.org>, Ryosuke Niwa  <rniwa@webkit.org>',
>+            [('Eric Seidel', 'eric@webkit.org'), ('Ryosuke Niwa', 'rniwa@webkit.org')])
>+        self._assert_parse_authors('Zan Dobersek  <zandobersek@gmail.com> and Philippe Normand  <pnormand@igalia.com>',
>+            [('Zan Dobersek', 'zandobersek@gmail.com'), ('Philippe Normand', 'pnormand@igalia.com')])
>+        self._assert_parse_authors('New Contributor  <new@webkit.org> and Noob  <noob@webkit.org>',
>+            [('New Contributor', 'new@webkit.org'), ('Noob', 'noob@webkit.org')])
>+
>     def _assert_has_valid_reviewer(self, reviewer_line, expected):
>         self.assertEqual(self._entry_with_reviewer(reviewer_line).has_valid_reviewer(), expected)
> 
>Index: Tools/Scripts/webkitpy/common/config/committers.py
>===================================================================
>--- Tools/Scripts/webkitpy/common/config/committers.py	(revision 100708)
>+++ Tools/Scripts/webkitpy/common/config/committers.py	(working copy)
>@@ -594,19 +594,19 @@ class CommitterList(object):
>         return contributorWithMinDistance, minDistance
> 
>     def account_by_login(self, login):
>-        return self._login_to_account_map().get(login.lower())
>+        return self._login_to_account_map().get(login.lower()) if login else None
> 
>     def account_by_email(self, email):
>-        return self._email_to_account_map().get(email.lower())
>+        return self._email_to_account_map().get(email.lower()) if email else None
> 
>     def contributor_by_name(self, name):
>-        return self._name_to_contributor_map().get(name.lower())
>+        return self._name_to_contributor_map().get(name.lower()) if name else None
> 
>     def contributor_by_email(self, email):
>-        return self._contributor_only(self.account_by_email(email))
>+        return self._contributor_only(self.account_by_email(email)) if email else None
> 
>     def committer_by_email(self, email):
>-        return self._committer_only(self.account_by_email(email))
>+        return self._committer_only(self.account_by_email(email)) if email else None
> 
>     def reviewer_by_email(self, email):
>-        return self._reviewer_only(self.account_by_email(email))
>+        return self._reviewer_only(self.account_by_email(email)) if email else None

Flags:

eric: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 72690: 115741