[webkit-reviews] review granted: [Bug 72351] Implement edit-distance based reviewer recognition algorithm : [Attachment 115270] Brought back forgotten editdistance.py
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 15 16:31:07 PST 2011
Eric Seidel <eric at webkit.org> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 72351: Implement edit-distance based reviewer recognition algorithm
https://bugs.webkit.org/show_bug.cgi?id=72351
Attachment 115270: Brought back forgotten editdistance.py
https://bugs.webkit.org/attachment.cgi?id=115270&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=115270&action=review
Looks great, as long as the reviewer() comment below isn't a bug.
> Tools/Scripts/webkitpy/common/editdistance.py:36
> +def edit_distance(str1, str2):
> + unsignedShort = 'h'
> + distances = [array(unsignedShort, (0,) * (len(str2) + 1)) for i in
range(0, len(str1) + 1)]
> + for i in range(1, len(str1) + 1):
> + distances[i][0] = i
You might want to add a comment here to explain what you're doing. In my 3rd
reading it's still not immediately clear what this does. A link to a wiki page
describing the algorithm might also be helpful.
> Tools/Scripts/webkitpy/common/checkout/changelog.py:150
> + def _fuzz_match_reviewers(self, reviewers_text_list):
> + if not reviewers_text_list:
Much nicer!
> Tools/Scripts/webkitpy/common/checkout/changelog.py:186
> - return self._reviewer # Might be None, might also not be a
Reviewer!
> + return self._reviewers[0] if len(self._reviewers) > 0 else None
So won't this sometimes return an array? If your flattening fails because
there are some lists of reviewers with more than one reviewer?
> Tools/Scripts/webkitpy/common/checkout/changelog.py:199
> + if re.search("unreviewed", self._contents, re.IGNORECASE):
> + return True
> + return False
Isn't this just bool(re.search...?
> Tools/Scripts/webkitpy/common/checkout/changelog_unittest.py:430
> + 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'], [])
Thank you. :)
> Tools/Scripts/webkitpy/common/config/committers.py:524
> + #FIXME: This should do case-insensitive comparison or assert
that all IRC nicknames are in lowercase
I think there is normaly a space between # and FIXME
> Tools/Scripts/webkitpy/common/config/committers.py:539
> + def contributors_by_fuzzy_match(self, string):
MIND. BLOWN. sheriff-bot whois is drooling. :)
More information about the webkit-reviews
mailing list