Diff
Modified: trunk/Tools/ChangeLog (100406 => 100407)
--- trunk/Tools/ChangeLog 2011-11-16 06:21:01 UTC (rev 100406)
+++ trunk/Tools/ChangeLog 2011-11-16 06:41:17 UTC (rev 100407)
@@ -1,3 +1,30 @@
+2011-11-14 Ryosuke Niwa <[email protected]>
+
+ Implement edit-distance based reviewer recognition algorithm
+ https://bugs.webkit.org/show_bug.cgi?id=72351
+
+ Reviewed by Eric Seidel.
+
+ Implement an algorithm to recognize reviewer's name based on its edit distance (or more precisely
+ its Levenshtein distance) to each reviewer's full name, first, last and middle names, and IRC nicknames.
+ Furthermore, we cap the maximum edit distance at len(name) - 1 to avoid matching a bogus string like
+ "build fix" to a reviewer's name (e.g. with with edit distance 9).
+
+ This algorithm is implemented in CommitterList.contributors_by_fuzzy_match. The function to compute
+ the edit distance is implemented in edit_distance.py.
+
+ Also moved _has_valid_reviewer from ValidateReviewer to ChangeLogEntry because we can no longer rely
+ on the presence of ChangeLogEntry.reviewer() to verify that reviewer string is nicely formatted.
+
+ * Scripts/webkitpy/common/checkout/changelog.py:
+ * Scripts/webkitpy/common/checkout/changelog_unittest.py:
+ * Scripts/webkitpy/common/config/committers.py:
+ * Scripts/webkitpy/common/config/committers_unittest.py:
+ * Scripts/webkitpy/common/editdistance.py: Added.
+ * Scripts/webkitpy/common/editdistance_unittest.py: Added.
+ * Scripts/webkitpy/tool/steps/validatereviewer.py:
+ * Scripts/webkitpy/tool/steps/validatereviewer_unittest.py: Removed.
+
2011-11-15 Tony Chang <[email protected]>
Skip a failing webkitpy test on cygwin.
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/changelog.py (100406 => 100407)
--- trunk/Tools/Scripts/webkitpy/common/checkout/changelog.py 2011-11-16 06:21:01 UTC (rev 100406)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/changelog.py 2011-11-16 06:41:17 UTC (rev 100407)
@@ -146,6 +146,13 @@
return reviewer_text, reviewer_list
+ def _fuzz_match_reviewers(self, reviewers_text_list):
+ if not reviewers_text_list:
+ return []
+ list_of_reviewers = [self._committer_list.contributors_by_fuzzy_match(reviewer)[0] for reviewer in reviewers_text_list]
+ # Flatten lists and get rid of any reviewers with more than one candidate.
+ return [reviewers[0] for reviewers in list_of_reviewers if len(reviewers) == 1]
+
def _parse_entry(self):
match = re.match(self.date_line_regexp, self._contents, re.MULTILINE)
if not match:
@@ -155,8 +162,8 @@
self._author_name = match.group("name") if match else None
self._author_email = match.group("email") if match else None
- self._reviewer_text, self._reviewer_list = ChangeLogEntry._parse_reviewer_text(self._contents)
- self._reviewer = self._committer_list.committer_by_name(self._reviewer_list[0]) if self._reviewer_list else None
+ 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._touched_files = re.findall(self.touched_files_regexp, self._contents, re.MULTILINE)
@@ -175,9 +182,21 @@
def reviewer_text(self):
return self._reviewer_text
+ # Might be None, might also not be a Reviewer!
def reviewer(self):
- return self._reviewer # Might be None, might also not be a Reviewer!
+ return self._reviewers[0] if len(self._reviewers) > 0 else None
+ def reviewers(self):
+ return self._reviewers
+
+ def has_valid_reviewer(self):
+ if self._reviewers_text_list:
+ for reviewer in self._reviewers_text_list:
+ reviewer = self._committer_list.committer_by_name(reviewer)
+ if reviewer:
+ return True
+ return bool(re.search("unreviewed", self._contents, re.IGNORECASE))
+
def contents(self):
return self._contents
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py (100406 => 100407)
--- trunk/Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py 2011-11-16 06:21:01 UTC (rev 100406)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py 2011-11-16 06:41:17 UTC (rev 100407)
@@ -377,41 +377,16 @@
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')
+ self.assertEquals(reviewer_list, ['Darin Adler'])
+ # 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'])
+
reviewer_text, reviewer_list = ChangeLogEntry._parse_reviewer_text('Reviewed by NOBODY.')
self.assertEquals(reviewer_text, None)
@@ -433,6 +408,48 @@
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 _entry_with_reviewer(self, reviewer_line):
+ return ChangeLogEntry('''2009-08-19 Eric Seidel <[email protected]>
+
+ REVIEW_LINE
+
+ * Scripts/bugzilla-tool:
+'''.replace("REVIEW_LINE", reviewer_line))
+
+ def _contributors(self, names):
+ return [CommitterList().contributor_by_name(name) for name in names]
+
+ def _assert_fuzzy_reviewer_match(self, reviewer_text, expected_text_list, expected_contributors):
+ unused, reviewer_text_list = ChangeLogEntry._parse_reviewer_text(reviewer_text)
+ self.assertEquals(reviewer_text_list, expected_text_list)
+ self.assertEquals(self._entry_with_reviewer(reviewer_text).reviewers(), self._contributors(expected_contributors))
+
+ def test_fuzzy_reviewer_match(self):
+ self._assert_fuzzy_reviewer_match('Reviewed by Dimitri Glazkov, build fix', ['Dimitri Glazkov', 'build fix'], ['Dimitri Glazkov'])
+ self._assert_fuzzy_reviewer_match('Reviewed by BUILD FIX', ['BUILD FIX'], [])
+ self._assert_fuzzy_reviewer_match('Reviewed by Mac build fix', ['Mac build fix'], [])
+ self._assert_fuzzy_reviewer_match('Reviewed by Darin Adler, Dan Bernstein, Adele Peterson, and others.',
+ ['Darin Adler', 'Dan Bernstein', 'Adele Peterson', 'others'], ['Darin Adler', 'Dan Bernstein', 'Adele Peterson'])
+ self._assert_fuzzy_reviewer_match('Reviewed by George Staikos (and others)', ['George Staikos', 'others'], ['George Staikos'])
+ self._assert_fuzzy_reviewer_match('Reviewed by Mark Rowe, but Dan Bernstein also reviewed and asked thoughtful questions.',
+ ['Mark Rowe', 'but Dan Bernstein also reviewed', 'asked thoughtful questions'], ['Mark Rowe'])
+ 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_has_valid_reviewer(self, reviewer_line, expected):
+ self.assertEqual(self._entry_with_reviewer(reviewer_line).has_valid_reviewer(), expected)
+
+ def test_has_valid_reviewer(self):
+ self._assert_has_valid_reviewer("Reviewed by Eric Seidel.", True)
+ self._assert_has_valid_reviewer("Reviewed by Eric Seidel", True) # Not picky about the '.'
+ self._assert_has_valid_reviewer("Reviewed by Eric.", False)
+ self._assert_has_valid_reviewer("Reviewed by Eric C Seidel.", False)
+ self._assert_has_valid_reviewer("Rubber-stamped by Eric.", False)
+ self._assert_has_valid_reviewer("Rubber-stamped by Eric Seidel.", True)
+ self._assert_has_valid_reviewer("Rubber stamped by Eric.", False)
+ self._assert_has_valid_reviewer("Rubber stamped by Eric Seidel.", True)
+ self._assert_has_valid_reviewer("Unreviewed build fix.", True)
+
def test_latest_entry_parse(self):
changelog_contents = u"%s\n%s" % (self._example_entry, self._example_changelog)
changelog_file = StringIO(changelog_contents)
Modified: trunk/Tools/Scripts/webkitpy/common/config/committers.py (100406 => 100407)
--- trunk/Tools/Scripts/webkitpy/common/config/committers.py 2011-11-16 06:21:01 UTC (rev 100406)
+++ trunk/Tools/Scripts/webkitpy/common/config/committers.py 2011-11-16 06:41:17 UTC (rev 100407)
@@ -29,6 +29,7 @@
#
# WebKit's Python module for committer and reviewer validation.
+from webkitpy.common.editdistance import edit_distance
class Account(object):
def __init__(self, name, email_or_emails, irc_nickname_or_nicknames=None):
@@ -449,7 +450,6 @@
Reviewer("Zoltan Herczeg", ["[email protected]", "[email protected]"], "zherczeg"),
]
-
class CommitterList(object):
# Committers and reviewers are passed in to allow easy testing
@@ -521,6 +521,7 @@
def contributor_by_irc_nickname(self, irc_nickname):
for contributor in self.contributors():
+ # FIXME: This should do case-insensitive comparison or assert that all IRC nicknames are in lowercase
if contributor.irc_nicknames and irc_nickname in contributor.irc_nicknames:
return contributor
return None
@@ -528,6 +529,39 @@
def contributors_by_search_string(self, string):
return filter(lambda contributor: contributor.contains_string(string), self.contributors())
+ def _tokenize_contributor_name(self, contributor):
+ full_name_in_lowercase = contributor.full_name.lower()
+ tokens = [full_name_in_lowercase] + full_name_in_lowercase.split()
+ if contributor.irc_nicknames:
+ return tokens + [nickname.lower() for nickname in contributor.irc_nicknames if len(nickname) > 5]
+ return tokens
+
+ def contributors_by_fuzzy_match(self, string):
+ string = string.lower()
+
+ # First path, optimitically match for fullname, email and irc_nicknames
+ account = self.account_by_email(string) or self.contributor_by_irc_nickname(string) or self.contributor_by_name(string)
+ if account:
+ return [account], 0
+
+ # Second path, much slower search using edit-distance
+ contributorWithMinDistance = []
+ minDistance = len(string) / 2 - 1
+ for contributor in self.contributors():
+ tokens = self._tokenize_contributor_name(contributor)
+ editdistances = [edit_distance(token, string) for token in tokens if abs(len(token) - len(string)) <= minDistance]
+ if not editdistances:
+ continue
+ distance = min(editdistances)
+ if distance == minDistance:
+ contributorWithMinDistance.append(contributor)
+ elif distance < minDistance:
+ contributorWithMinDistance = [contributor]
+ minDistance = distance
+ if not len(contributorWithMinDistance):
+ return [], len(string)
+ return contributorWithMinDistance, minDistance
+
def account_by_login(self, login):
return self._login_to_account_map().get(login.lower())
Modified: trunk/Tools/Scripts/webkitpy/common/config/committers_unittest.py (100406 => 100407)
--- trunk/Tools/Scripts/webkitpy/common/config/committers_unittest.py 2011-11-16 06:21:01 UTC (rev 100406)
+++ trunk/Tools/Scripts/webkitpy/common/config/committers_unittest.py 2011-11-16 06:41:17 UTC (rev 100407)
@@ -91,3 +91,35 @@
self.assertEqual(committer_list.contributors_by_search_string('test'), [contributor, committer, reviewer])
self.assertEqual(committer_list.contributors_by_search_string('rad'), [reviewer])
self.assertEqual(committer_list.contributors_by_search_string('Two'), [reviewer])
+
+ def _assert_fuzz_match(self, text, name_of_expected_contributor, expected_distance):
+ committers = CommitterList()
+ expected_contributors = [committers.contributor_by_name(name_of_expected_contributor)] if name_of_expected_contributor else []
+ self.assertEqual(committers.contributors_by_fuzzy_match(text), (expected_contributors, expected_distance))
+
+ def test_contributors_by_fuzzy_match(self):
+ self._assert_fuzz_match('Geoff Garen', 'Geoffrey Garen', 3)
+ self._assert_fuzz_match('Kenneth Christiansen', 'Kenneth Rohde Christiansen', 6)
+ self._assert_fuzz_match('Ken Russell', 'Kenneth Russell', 4)
+ self._assert_fuzz_match('Dave Hyatt', 'David Hyatt', 2)
+ self._assert_fuzz_match('Dave Kilzer', 'David Kilzer', 2)
+ self._assert_fuzz_match('Antti "printf" Koivisto', 'Antti Koivisto', 9)
+ self._assert_fuzz_match('Sammy Weinig', 'Sam Weinig', 2)
+ self._assert_fuzz_match('Andres Kling', 'Andreas Kling', 1)
+ self._assert_fuzz_match('Darin Adler\'', 'Darin Adler', 1)
+ self._assert_fuzz_match('Joe Pecoraro', 'Joseph Pecoraro', 3)
+ self._assert_fuzz_match('Dr Dan Bernstein', 'Dan Bernstein', 3)
+ self._assert_fuzz_match('Mitzpettel', 'Dan Bernstein', 0)
+ self._assert_fuzz_match('[email protected]', 'Ryosuke Niwa', 0)
+ self._assert_fuzz_match('Ap', 'Alexey Proskuryakov', 0)
+ self._assert_fuzz_match('Sam', 'Sam Weinig', 0)
+ self._assert_fuzz_match('darin', 'Darin Adler', 0)
+ self._assert_fuzz_match('harrison', 'David Harrison', 0)
+ self._assert_fuzz_match('others', None, 6)
+ self._assert_fuzz_match('BUILD FIX', None, 9)
+ self._assert_fuzz_match('but Dan Bernstein also reviewed', None, 31)
+ self._assert_fuzz_match('asked thoughtful questions', None, 26)
+ self._assert_fuzz_match('build fix of mac', None, 16)
+ self._assert_fuzz_match('a spell checker', None, 15)
+ self._assert_fuzz_match('nobody, build fix', None, 17)
+ self._assert_fuzz_match('NOBODY (chromium build fix)', None, 27)
Added: trunk/Tools/Scripts/webkitpy/common/editdistance.py (0 => 100407)
--- trunk/Tools/Scripts/webkitpy/common/editdistance.py (rev 0)
+++ trunk/Tools/Scripts/webkitpy/common/editdistance.py 2011-11-16 06:41:17 UTC (rev 100407)
@@ -0,0 +1,47 @@
+# Copyright (c) 2011 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+from array import array
+
+
+def edit_distance(str1, str2):
+ unsignedShort = 'h'
+ distances = [array(unsignedShort, (0,) * (len(str2) + 1)) for i in range(0, len(str1) + 1)]
+ # distances[0][0] = 0 since distance between str1[:0] and str2[:0] is 0
+ for i in range(1, len(str1) + 1):
+ distances[i][0] = i # Distance between str1[:i] and str2[:0] is i
+
+ for j in range(1, len(str2) + 1):
+ distances[0][j] = j # Distance between str1[:0] and str2[:j] is j
+
+ for i in range(0, len(str1)):
+ for j in range(0, len(str2)):
+ diff = 0 if str1[i] == str2[j] else 1
+ # Deletion, Insertion, Identical / Replacement
+ distances[i + 1][j + 1] = min(distances[i + 1][j] + 1, distances[i][j + 1] + 1, distances[i][j] + diff)
+ return distances[len(str1)][len(str2)]
Added: trunk/Tools/Scripts/webkitpy/common/editdistance_unittest.py (0 => 100407)
--- trunk/Tools/Scripts/webkitpy/common/editdistance_unittest.py (rev 0)
+++ trunk/Tools/Scripts/webkitpy/common/editdistance_unittest.py 2011-11-16 06:41:17 UTC (rev 100407)
@@ -0,0 +1,43 @@
+# Copyright (c) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import unittest
+
+from webkitpy.common.editdistance import edit_distance
+
+
+class EditDistanceTest(unittest.TestCase):
+ def test_edit_distance(self):
+ self.assertEqual(edit_distance('', 'aa'), 2)
+ self.assertEqual(edit_distance('aa', ''), 2)
+ self.assertEqual(edit_distance('a', 'ab'), 1)
+ self.assertEqual(edit_distance('ab', 'a'), 1)
+ self.assertEqual(edit_distance('ab', 'aa'), 1)
+ self.assertEqual(edit_distance('aa', 'ab'), 1)
+ self.assertEqual(edit_distance('abd', 'abcdef'), 3)
+ self.assertEqual(edit_distance('abcdef', 'abd'), 3)
Modified: trunk/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py (100406 => 100407)
--- trunk/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py 2011-11-16 06:21:01 UTC (rev 100406)
+++ trunk/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py 2011-11-16 06:41:17 UTC (rev 100407)
@@ -43,16 +43,6 @@
Options.non_interactive,
]
- # FIXME: This should probably move onto ChangeLogEntry
- def _has_valid_reviewer(self, changelog_entry):
- if changelog_entry.reviewer():
- return True
- if re.search("unreviewed", changelog_entry.contents(), re.IGNORECASE):
- return True
- if re.search("rubber[ -]stamp", changelog_entry.contents(), re.IGNORECASE):
- return True
- return False
-
def run(self, state):
# FIXME: For now we disable this check when a user is driving the script
# this check is too draconian (and too poorly tested) to foist upon users.
@@ -60,7 +50,7 @@
return
for changelog_path in self.cached_lookup(state, "changelogs"):
changelog_entry = ChangeLog(changelog_path).latest_entry()
- if self._has_valid_reviewer(changelog_entry):
+ if changelog_entry.has_valid_reviewer():
continue
reviewer_text = changelog_entry.reviewer_text()
if reviewer_text:
Deleted: trunk/Tools/Scripts/webkitpy/tool/steps/validatereviewer_unittest.py (100406 => 100407)
--- trunk/Tools/Scripts/webkitpy/tool/steps/validatereviewer_unittest.py 2011-11-16 06:21:01 UTC (rev 100406)
+++ trunk/Tools/Scripts/webkitpy/tool/steps/validatereviewer_unittest.py 2011-11-16 06:41:17 UTC (rev 100407)
@@ -1,57 +0,0 @@
-# Copyright (C) 2010 Google Inc. All rights reserved.
-#
-# Redistribution and use in source and binary forms, with or without
-# modification, are permitted provided that the following conditions are
-# met:
-#
-# * Redistributions of source code must retain the above copyright
-# notice, this list of conditions and the following disclaimer.
-# * Redistributions in binary form must reproduce the above
-# copyright notice, this list of conditions and the following disclaimer
-# in the documentation and/or other materials provided with the
-# distribution.
-# * Neither the name of Google Inc. nor the names of its
-# contributors may be used to endorse or promote products derived from
-# this software without specific prior written permission.
-#
-# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-import unittest
-
-from webkitpy.common.checkout.changelog import ChangeLogEntry
-from webkitpy.common.system.outputcapture import OutputCapture
-from webkitpy.tool.mocktool import MockOptions, MockTool
-from webkitpy.tool.steps.validatereviewer import ValidateReviewer
-
-class ValidateReviewerTest(unittest.TestCase):
- _boilerplate_entry = '''2009-08-19 Eric Seidel <[email protected]>
-
- REVIEW_LINE
-
- * Scripts/bugzilla-tool:
-'''
-
- def _test_review_text(self, step, text, expected):
- contents = self._boilerplate_entry.replace("REVIEW_LINE", text)
- entry = ChangeLogEntry(contents)
- self.assertEqual(step._has_valid_reviewer(entry), expected)
-
- def test_has_valid_reviewer(self):
- step = ValidateReviewer(MockTool(), MockOptions())
- self._test_review_text(step, "Reviewed by Eric Seidel.", True)
- self._test_review_text(step, "Reviewed by Eric Seidel", True) # Not picky about the '.'
- self._test_review_text(step, "Reviewed by Eric.", False)
- self._test_review_text(step, "Reviewed by Eric C Seidel.", False)
- self._test_review_text(step, "Rubber-stamped by Eric.", True)
- self._test_review_text(step, "Rubber stamped by Eric.", True)
- self._test_review_text(step, "Unreviewed build fix.", True)
