[Webkit-unassigned] [Bug 25884] WebKit needs a style linting tool

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=25884





--- Comment #35 from Shinichiro Hamaji <hamaji at chromium.org>  2009-07-14 02:40:53 PDT ---
Thanks for the thorough comments on this big patch! I've fixed most of them.
Some comments will be inlined.

> A single line to separate the functions is sufficient (though two lines before
> a class of course).

PEP0008 is saying that we should have two-lines for top-level functions:
http://www.python.org/dev/peps/pep-0008/

> THREADING_LIST = (
> 
> Since it is a constant.

I changed this to _THREADING_LIST as this is private.

> I would unwrap this line.

I also unwrapped some lines which were wrapped around operators.

> I don't think the whitespace/comments is correct for WebKit right now.  WebKit
> style uses 1 space before end of line comments (unliks Google style which does
> 2 spaces), so I'd just remove this one until we fix it.

This is the list of features we are disabling, not enabling. So, the original
code would be OK. I added some comments to clarify what we are doing here.
Thanks.

> As I've looked through this code, I really want it to get landed but I'm
> concerned there are issues with respect to WebKit style that will have to be
> fixed after it is landed when multiple people can contribute and we can do
> incremental changes to improve it.  Given that it is a tool and not part of
> this shipping code, I think that it is ok to handle in this fashion.
> 
> However, it would be nice to let people know about the potential alpha state of
> the warnings (due to the adaption to WebKit style).
> 
> In short for now, it would be nice to print a warning banner here that this
> tool is still under development and may flag things incorrectly.  After it is
> landed and people have an opportunity to try it out we can remove the warning
> banner.

Agreed. I've also added some comments which mention that this python script may
be inconsistent with other WebKit's script as this comes from Google's cpplint.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.