[webkit-reviews] review granted: [Bug 25884] WebKit needs a style linting tool : [Attachment 32703] first cpplint v3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jul 14 07:53:03 PDT 2009


David Levin <levin at chromium.org> has granted Shinichiro Hamaji
<hamaji at chromium.org>'s request for review:
Bug 25884: WebKit needs a style linting tool
https://bugs.webkit.org/show_bug.cgi?id=25884

Attachment 32703: first cpplint v3
https://bugs.webkit.org/attachment.cgi?id=32703&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Here's a few last changes.  I'm willing to do them while landing the patch, but
if you put up a new one that fixes them, that would also be nice.


> diff --git a/WebKitTools/Scripts/cpplint.py b/WebKitTools/Scripts/cpplint.py
> +# ********************* WARNING WARNING WARNING *********************
> +#
> +# This tool is in the process of development and may give inaccurate
> +# results at resent.  Please files bugs (and/or patches) for things
> +# that you notice that it flags incorrectly.

I actually meant this as something that the tool printed out to warn anyone who
tried it out.

> +#
> +# Also, please note that the style of this Python script may be
> +# inconsistent with other Python scripts in WebKit.
> +#

I don't think this is necessary.  I've tried to be very careful with the style
in this patch and you've done a lot of changes to make it conform.

> +	   self.filters = _DEFAULT_FILTERS[:]
> +	   for filt in filters.split(','):

s/filt/filter/

> +	       clean_filt = filt.strip()

s/clean_filt/clean_filter/

> +	   trigger = base_trigger * 2**_verbose_level()

Add spaces around **


> +_RE_PATTERN_IVALID_INCREMENT = re.compile(

s/IVLIAD/INVALID/

> +    fncall = line	# if there's no control flow construct, look at whole
line

s/fncall/function_call/

> +	       if search(r'(;|})', start_line):  # Declarations and trivial
functions
> +		   body_found = True
> +		   break			      # ... ignore
> +	       elif search(r'{', start_line):

This should be an "if"	(This is in the same spirit as when the previous if
ends with a return.)

> +	   prevbrace = previous_line.rfind('{')

s/prevbrace/previous_brace/

> +def update_include_state(filename, include_state, io=codecs):
...
> +    headerfile = None

header_file


More information about the webkit-reviews mailing list