[webkit-reviews] review denied: [Bug 17124] Use a script to make style more consistent throughout WebCore/JavaScriptCore : [Attachment 18834] full patch

[bugzilla-daemon at webkit.org]

Alp Toker <alp at atoker.com> has denied Eric Seidel <eric at webkit.org>'s request
for review:
Bug 17124: Use a script to make style more consistent throughout
WebCore/JavaScriptCore
http://bugs.webkit.org/show_bug.cgi?id=17124

Attachment 18834: full patch
http://bugs.webkit.org/attachment.cgi?id=18834&action=edit

------- Additional Comments from Alp Toker <alp at atoker.com>
The NULLs in the GTK+ part of WebCore probably shouldn't be changed. NULL
parameters are common in GObject and using 0 makes calls difficult to read.
This is similar to the coding style exceptions made in the Mac Obj-C code.

I haven't worked with Win32 for some years but in my experience Win32 hackers
will prefer to use NULL when calling into Win32, so you might want to avoid the
s/NULL/0 cleanup for those too.


Some things the script missed:

Weird whitespace still there after second ):

-    switch ( readResult)    {
+    switch (readResult)    {

Position of first & not fixed:

-ImageDecoderQt::ReadContext::ReadContext(const IncomingData & data, LoadMode
loadMode, ImageList &target)
+ImageDecoderQt::ReadContext::ReadContext(const IncomingData & data, LoadMode
loadMode, ImageList& target)


Whitespace not fixed entirely:

-  gettimeofday( &aTimeval, &aTimezone );
-  return (double)aTimeval.tv_sec + (double)(aTimeval.tv_usec / 1000000.0 );
+  gettimeofday( &aTimeval, &aTimezone);
+  return (double)aTimeval.tv_sec + (double)(aTimeval.tv_usec / 1000000.0);

I didn't keep looking, can do a more full review if you need.

Maybe a combination of astyle and your tool will help catch more cases? It
certainly makes sense to do as much cleanup as we can in one go.

I remember astyle caught some of the cases yours doesn't, and yours covers some
cases that astyle is bad with. There's also WebKitTools/Scripts/wkstyle which
may have some ideas.

Thanks for looking into this Eric!