Modified: trunk/Tools/Scripts/webkitpy/common/checkout/changelog.py (100232 => 100233)
--- trunk/Tools/Scripts/webkitpy/common/checkout/changelog.py 2011-11-15 02:46:35 UTC (rev 100232)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/changelog.py 2011-11-15 03:08:16 UTC (rev 100233)
@@ -83,13 +83,28 @@
reviewed_byless_regexp = r'^\s*((Review|Rubber(\s*|-)stamp)(s|ed)?|RS)(\s+|\s*=\s*)(?P<reviewer>([A-Z]\w+\s*)+)[\.,]?\s*$'
- contributor_name_noise_regexp = re.compile(r"""
- (\s+(landed|committed|)\s+by.+) # landed by, commented by, etc...
- |\..+ # text afetr the first period (inclusive)
+ reviewer_name_noise_regexp = re.compile(r"""
+ (\s+((tweaked\s+)?and\s+)?(landed|committed|okayed)\s+by.+) # "landed by", "commented by", etc...
+ |(^(Reviewed\s+)?by\s+) # extra "Reviewed by" or "by"
+ |\.(?:(\s.+|$)) # text after the first period followed by a space
|([(<]\s*[\w_\-\.]+@[\w_\-\.]+[>)]) # email addresses
- |((?<=and)\s+([a-z\-]+\s+)+by) # phrases like "given a glance-over by" and "looked over by" (no capital letters)
+ |([(<](https?://?bugs.)webkit.org[^>)]+[>)]) # bug url
+ |("[^"]+") # wresler names like 'Sean/Shawn/Shaun' in 'Geoffrey "Sean/Shawn/Shaun" Garen'
+ |('[^']+') # wresler names like "The Belly" in "Sam 'The Belly' Weinig"
+ |((Mr|Ms|Dr|Mrs|Prof)\.(\s+|$))
""", re.IGNORECASE | re.VERBOSE)
+ reviewer_name_casesensitive_noise_regexp = re.compile(r"""
+ ((\s+|^)(and\s+)?([a-z-]+\s+){5,}by\s+) # e.g. "and given a good once-over by"
+ |(\(\s*(?!(and|[A-Z])).+\)) # any parenthesis that doesn't start with "and" or a capital letter
+ |(with(\s+[a-z-]+)+) # phrases with "with no hesitation" in "Sam Weinig with no hesitation"
+ """, re.VERBOSE)
+
+ nobody_regexp = re.compile(r"""(\s+|^)nobody(
+ ((,|\s+-)?\s+(\w+\s+)+fix.*) # e.g. nobody, build fix...
+ |(\s*\([^)]+\).*) # NOBODY (..)...
+ |$)""", re.IGNORECASE | re.VERBOSE)
+
# e.g. == Rolled over to ChangeLog-2011-02-16 ==
rolled_over_regexp = r'^== Rolled over to ChangeLog-\d{4}-\d{2}-\d{2} ==$'
@@ -113,19 +128,24 @@
reviewer_text = match.group("reviewer")
- reviewer_text = ChangeLogEntry.contributor_name_noise_regexp.sub('', reviewer_text)
+ reviewer_text = ChangeLogEntry.nobody_regexp.sub('', reviewer_text)
+ reviewer_text = ChangeLogEntry.reviewer_name_noise_regexp.sub('', reviewer_text)
+ reviewer_text = ChangeLogEntry.reviewer_name_casesensitive_noise_regexp.sub('', reviewer_text)
+ reviewer_text = reviewer_text.replace('(', '').replace(')', '')
reviewer_text = re.sub(r'\s\s+|[,.]\s*$', ' ', reviewer_text).strip()
+ 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 = re.split(r'\s*(?:(?:,(?:\s+and\s+|&)?)|(?:and\s+|&)|(?:[/+]))\s*', reviewer_text)
# Get rid of "reviewers" like "even though this is just a..." in "Reviewed by Sam Weinig, even though this is just a..."
- reviewer_list = [reviewer for reviewer in reviewer_list if len(reviewer.split()) <= 5]
+ # and "who wrote the original code" in "Noam Rosenthal, who wrote the original code"
+ reviewer_list = [reviewer for reviewer in reviewer_list if not re.match('^who\s|^([a-z]+(\s+|\.|$)){6,}$', reviewer)]
return reviewer_text, reviewer_list
-
def _parse_entry(self):
match = re.match(self.date_line_regexp, self._contents, re.MULTILINE)
if not match:
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py (100232 => 100233)
--- trunk/Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py 2011-11-15 02:46:35 UTC (rev 100232)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py 2011-11-15 03:08:16 UTC (rev 100233)
@@ -313,7 +313,6 @@
self.assertEquals(reviewer_list, ['Alexey Proskuryakov'])
reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Sam Weinig, and given a good once-over by Jeff Miller.')
- self.assertEquals(reviewer_text, 'Sam Weinig, and Jeff Miller')
self.assertEquals(reviewer_list, ['Sam Weinig', 'Jeff Miller'])
reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed and landed by Brady Eidson')
@@ -323,6 +322,117 @@
reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text(' Reviewed by Sam Weinig, even though this is just a...')
self.assertEquals(reviewer_list, ['Sam Weinig'])
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by [email protected].')
+ self.assertEquals(reviewer_text, '[email protected]')
+ self.assertEquals(reviewer_list, ['[email protected]'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Dirk Schulze / Darin Adler.')
+ self.assertEquals(reviewer_text, 'Dirk Schulze / Darin Adler')
+ self.assertEquals(reviewer_list, ['Dirk Schulze', 'Darin Adler'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Sam Weinig + Oliver Hunt.')
+ self.assertEquals(reviewer_text, 'Sam Weinig + Oliver Hunt')
+ self.assertEquals(reviewer_list, ['Sam Weinig', 'Oliver Hunt'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Sam Weinig + Oliver Hunt.')
+ self.assertEquals(reviewer_text, 'Sam Weinig + Oliver Hunt')
+ self.assertEquals(reviewer_list, ['Sam Weinig', 'Oliver Hunt'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Rubber stamped by by Gustavo Noronha Silva')
+ self.assertEquals(reviewer_text, 'Gustavo Noronha Silva')
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Rubberstamped by Noam Rosenthal, who wrote the original code.')
+ self.assertEquals(reviewer_list, ['Noam Rosenthal'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Dan Bernstein (relanding of r47157)')
+ self.assertEquals(reviewer_list, ['Dan Bernstein'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Geoffrey "Sean/Shawn/Shaun" Garen')
+ self.assertEquals(reviewer_list, ['Geoffrey Garen'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Dave "Messy" Hyatt.')
+ self.assertEquals(reviewer_list, ['Dave Hyatt'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Sam \'The Belly\' Weinig')
+ self.assertEquals(reviewer_list, ['Sam Weinig'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Rubber-stamped by David "I\'d prefer not" Hyatt.')
+ self.assertEquals(reviewer_list, ['David Hyatt'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Mr. Geoffrey Garen.')
+ self.assertEquals(reviewer_list, ['Geoffrey Garen'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Darin (ages ago)')
+ self.assertEquals(reviewer_list, ['Darin'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Sam Weinig (except for a few comment and header tweaks).')
+ self.assertEquals(reviewer_list, ['Sam Weinig'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Sam Weinig (all but the FormDataListItem rename)')
+ self.assertEquals(reviewer_list, ['Sam Weinig'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Darin Adler, tweaked and landed by Beth.')
+ self.assertEquals(reviewer_list, ['Darin Adler'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Sam Weinig with no hesitation')
+ self.assertEquals(reviewer_list, ['Sam Weinig'])
+
+ # For now, we let unofficial reviewers recognized as reviewers
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Sam Weinig, Anders Carlsson, and (unofficially) Adam Barth.')
+ self.assertEquals(reviewer_list, ['Sam Weinig', 'Anders Carlsson', 'Adam Barth'])
+
+ # It's okay to have 'build fix' and 'others', etc... as a reviewer in the following cases because fuzzy-match would reject it
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Dimitri Glazkov, build fix')
+ self.assertEquals(reviewer_list, ['Dimitri Glazkov', 'build fix'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by BUILD FIX')
+ self.assertEquals(reviewer_list, ['BUILD FIX'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Mac build fix')
+ self.assertEquals(reviewer_list, ['Mac build fix'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Darin Adler, Dan Bernstein, Adele Peterson, and others.')
+ self.assertEquals(reviewer_text, 'Darin Adler, Dan Bernstein, Adele Peterson, and others')
+ self.assertEquals(reviewer_list, ['Darin Adler', 'Dan Bernstein', 'Adele Peterson', 'others'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by George Staikos (and others)')
+ self.assertEquals(reviewer_list, ['George Staikos', 'others'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Oliver Hunt, okayed by Darin Adler.')
+ self.assertEquals(reviewer_list, ['Oliver Hunt'])
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Mark Rowe, but Dan Bernstein also reviewed and asked thoughtful questions.')
+ self.assertEquals(reviewer_list, ['Mark Rowe', 'but Dan Bernstein also reviewed', 'asked thoughtful questions'])
+
+ # It's okay to have " in" and "by ", etc... in the following cases because we're going to fuzzy-match them later
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Darin Adler in <https://bugs.webkit.org/show_bug.cgi?id=47736>.')
+ self.assertEquals(reviewer_text, 'Darin Adler in')
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Adam Barth.:w')
+ self.assertEquals(reviewer_text, 'Adam Barth.:w')
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by Darin Adler).')
+ self.assertEquals(reviewer_text, 'Darin Adler')
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by NOBODY.')
+ self.assertEquals(reviewer_text, None)
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by NOBODY - Build Fix.')
+ self.assertEquals(reviewer_text, None)
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by NOBODY, layout tests fix.')
+ self.assertEquals(reviewer_text, None)
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by NOBODY (Qt build fix pt 2).')
+ self.assertEquals(reviewer_text, None)
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by NOBODY(rollout)')
+ self.assertEquals(reviewer_text, None)
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by NOBODY (Build fix, forgot to svn add this file)')
+ self.assertEquals(reviewer_text, None)
+
+ reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by nobody (trivial follow up fix), Joseph Pecoraro LGTM-ed.')
+ self.assertEquals(reviewer_text, None)
+
def test_latest_entry_parse(self):
changelog_contents = u"%s\n%s" % (self._example_entry, self._example_changelog)
changelog_file = StringIO(changelog_contents)
