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

[bugzilla-daemon at webkit.org]

Shinichiro Hamaji <hamaji at chromium.org> has denied Anders Bakken
<agbakken at gmail.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 57968: Patch
https://bugs.webkit.org/attachment.cgi?id=57968&action=review

------- Additional Comments from Shinichiro Hamaji <hamaji at chromium.org>
Yeah, I think we want some test cases for this change.

WebKitTools/Scripts/webkitpy/style/checkers/cpp.py:224
 +	def check_next_include_order(self, header_type, file_is_header,
include_state):
I think we can just use self as include_state?

WebKitTools/Scripts/webkitpy/style/checkers/cpp.py:2875
 +	for line_number in xrange(clean_lines.num_lines()):
It's unfortunate we need to iterate all lines twice. Cannot we implement this
by one-pass approach? Maybe it should be like

1. If primary header comes before config.h, emit a warning when we find
config.h
2. If primary header isn't found between config.h and "other headers", set
has_primary_header=False
3. If primary header is found after we see at least one "other headers", emit a
warning.