[webkit-reviews] review denied: [Bug 39514] check-webkit-style shouldn't complain about not including a primary header file if none exists. : [Attachment 89691] Simple fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 14 17:25:43 PDT 2011


David Levin <levin at chromium.org> has denied Dmitry Lomov <dslomov at google.com>'s
request for review:
Bug 39514: check-webkit-style shouldn't complain about not including a primary
header file if none exists.
https://bugs.webkit.org/show_bug.cgi?id=39514

Attachment 89691: Simple fix
https://bugs.webkit.org/attachment.cgi?id=89691&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=89691&action=review

What the stylebot said plus a few other things to consider.

> Tools/Scripts/webkitpy/style/checkers/cpp.py:315
> +    def check_next_include_order(self, header_type, file_is_header,
has_primary_header):

has_primary_header seems ambiguous to me.

What about primary_header_exists?

>> Tools/Scripts/webkitpy/style/checkers/cpp.py:2600
>> +def _has_primary_header(filename):
> 
> expected 2 blank lines, found 1  [pep8/E302] [5]

Consider: _does_primary_header_exist()

> Tools/Scripts/webkitpy/style/checkers/cpp.py:2602
> +    if the file is not source file or primary header does not exist

Add .

> Tools/Scripts/webkitpy/style/checkers/cpp.py:2608
> +    if os.path.isfile(primary_header):

You could just 
  return os.path.isfile(primary_header)

> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:2592
> +	   # File with non-existing primary header should not produce errors

Add . after comments (and other places).


More information about the webkit-reviews mailing list