[Webkit-unassigned] [Bug 56989] webkit-patch should be more intelligent about whether a bug applies to a patch

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=56989


Andrew Foster <andrewf at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrewf at chromium.org




--- Comment #5 from Andrew Foster <andrewf at chromium.org>  2011-03-24 21:50:39 PST ---
> > Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py:53
> > +def parse_bug_id(message, show_bug_url_on_own_line=False):
> 
> I don't see why you need this argument. As best I can tell, all the paces where this is used are to get the bug ID out of a changelog/commit entry and they should all have the same format. Is there a case that breaks if you just always restrict to searching for the bug url on it's own line?

Currently sheriff_unittest.py creates a string "Created bug https://bugs.webkit.org/..." which is used as an input to parse_bug_id().  

I could alter this particular test, but my concern is potentially breaking anything that may be lacking robust unittests that assumes that a non-start-of-line match of a bug URL is handled by parse_bug_id()

> > Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py:59
> > +    if show_bug_url_on_own_line == False:
> 
> If you invert the order here, you can just check "if show_bug_url_on_own_line". Also, general preferred style is to use "if bool" or "if not bool" instead of comparing to True/False.

Fixed, thanks, I'll upload another patch if we agree that an argument is or isn't needed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.