[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