Bug 72090 – Make ChangeLogEntry's reviewer parsing algorithm support last 4 WebCore change logs

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Ryosuke Niwa]

Significantly improve ChangeLogEntry's reviewer parsing algorithm. This version can successfully parse the following change logs in Source/WebCore/

ChangeLog, ChangeLog-2011-10-19, ChangeLog-2011-06-04, ChangeLog-2011-02-16, ChangeLog-2010-12-06

[Ryosuke Niwa]

Created attachment 114628 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 114628 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114628&action=review

> Tools/Scripts/webkitpy/common/checkout/changelog.py:88
> +    reviewed_by_regexp = r'^\s*((\w+\s+)+and\s+)?(Review|Rubber(\s*|-)stamp)(s|ed)?\s+([a-z]+\s+)*?by\s+(?P<reviewer>.*?)[\.,]?\s*$'
> +
> +    reviewed_byless_regexp = r'^\s*((Review|Rubber(\s*|-)stamp)(s|ed)?|RS)(\s+|\s*=\s*)(?P<reviewer>([A-Z]\w+\s*)+)[\.,]?\s*$'
> +
> +    # e.g. "landed by", email address, and phrases like "given a glance-over by" and "looked over by"
> +    contributor_name_noise_regexp = re.compile(r'(\s+(landed|committed|)\s+by.+)|\..+'
> +        + r'|([(<]\s*[\w_\-\.]+@[\w_\-\.]+[>)])|((?<=and)\s+([a-z\-]+\s+)+by)', re.IGNORECASE)

I wish I didn't have to write such ugly regular expressions :( I'm more than happy to explain what they do

[Ryosuke Niwa]

Comment on attachment 114628 [details]
Patch

Some webkitpy tests are failing...

[Ryosuke Niwa]

Created attachment 114631 [details]
fixed the test

[Eric Seidel (no email)]

Comment on attachment 114631 [details]
fixed the test

LGTM.  You should also be aware of bug 26533.

[Tony Chang]

Comment on attachment 114628 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114628&action=review

>> Tools/Scripts/webkitpy/common/checkout/changelog.py:88
>> +        + r'|([(<]\s*[\w_\-\.]+@[\w_\-\.]+[>)])|((?<=and)\s+([a-z\-]+\s+)+by)', re.IGNORECASE)
> 
> I wish I didn't have to write such ugly regular expressions :( I'm more than happy to explain what they do

When I have long regular expressions, I like to use re.VERBOSE and add comments to each part.  There's a small example in the verbose section:
  http://docs.python.org/library/re.html
Or here's an example in perl:
  http://www.perl.com/pub/2004/01/16/regexps.html

[Ryosuke Niwa]

(In reply to comment #6)
> When I have long regular expressions, I like to use re.VERBOSE and add comments to each part.  There's a small example in the verbose section:
>   http://docs.python.org/library/re.html

That's a good idea! Will do before landing it.

[Ryosuke Niwa]

(In reply to comment #5)
> (From update of attachment 114631 [details])
> LGTM.  You should also be aware of bug 26533.

Yup, in fact, this is marked as a blocker of the bug 26533.

[Ryosuke Niwa]

Landed as http://trac.webkit.org/changeset/100002.