Bug 57184 – check-webkit-style should check ChangeLog for a valid bug number

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Patrick R. Gansterer]

check-webkit-style should check ChangeLog for a valid bug number

[Patrick R. Gansterer]

Created attachment 87074 [details]
Patch

[David Levin]

Comment on attachment 87074 [details]
Patch

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

Just a few things to address and this should be good to go!

Thanks.

> Tools/Scripts/webkitpy/style/checkers/changelog.py:13
> +#     * Neither the name of Google Inc. nor the names of its

I'm not sure if you want this 3rd clause in here, but if you want to keep it, whatever :).

> Tools/Scripts/webkitpy/style/checkers/changelog.py:42
> +        self.complain_bug_numer = False

typo: numer
Also this seems completely unused, so I would just remove it.

> Tools/Scripts/webkitpy/style/checkers/changelog.py:50
> +            line_number += entry_line_number

It looks like line_number is never used, so you can remove it and the enumerate call here.

> Tools/Scripts/webkitpy/style/checkers/changelog.py:52
> +            if re.search("https?://bugs.webkit.org/show_bug\.cgi\?id=\d+|http://webkit.org/b/\d+", line):

Consider using parse_bug_id from webkitpy/common/net/bugzilla/bugzilla.py instead.

> Tools/Scripts/webkitpy/style/checkers/changelog.py:58
> +

Consider using a for else pattern here which feels like it fits better:

for ...
    if blah:
       break
else:
   error()

> Tools/Scripts/webkitpy/style/checkers/changelog.py:61
> +                                            "changelog/bugnumber", 5,

I'd align with the ( from the previous line.

> Tools/Scripts/webkitpy/style/checkers/changelog.py:62
> +                                            "ChaneLog entry has no bug number")

typo: ChaneLog
 (and the alignement issue).

> Tools/Scripts/webkitpy/style/checkers/changelog_unittest.py:29
> +

Why the blank line?

> Tools/Scripts/webkitpy/style/checkers/changelog_unittest.py:44
> +            self.had_error = True

Where does this get set back to false?

> Tools/Scripts/webkitpy/style/checkers/changelog_unittest.py:76
> +            '2011-01-01  Patrick Gansterer  <[e-mail]>\n\n        Unreview build fix for r12345.\n',

Please add a test case like this: https://bugs.webkit.org/attachment.cgi?id=74800&action=prettypatch

[Patrick R. Gansterer]

Created attachment 87096 [details]
Patch

[David Levin]

Comment on attachment 87096 [details]
Patch

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

Awesome. Thanks!

> Tools/Scripts/webkitpy/style/checkers/changelog.py:43
> +        for line_number, line in enumerate(entry_lines):

get rid of enumerate here.

> Tools/Scripts/webkitpy/style/checkers/changelog.py:63
> +                if line_number == 0:

get rid of comparison to 0.

[Patrick R. Gansterer]

Committed r82083: <http://trac.webkit.org/changeset/82083>

[David Kilzer (:ddkilzer)]

*** Bug 37943 has been marked as a duplicate of this bug. ***