[webkit-reviews] review denied: [Bug 112638] [GTK] Add support for building the WebCore bindings to the gyp build : [Attachment 194118] Patch

[bugzilla-daemon at webkit.org]

Nico Weber <thakis at chromium.org> has denied Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 112638: [GTK] Add support for building the WebCore bindings to the gyp
build
https://bugs.webkit.org/show_bug.cgi?id=112638

Attachment 194118: Patch
https://bugs.webkit.org/attachment.cgi?id=194118&action=review

------- Additional Comments from Nico Weber <thakis at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=194118&action=review


Only one real question (about the chromium build), and a few small nits.

> Source/WebCore/WebCore.gyp/MakeNames.gypi:1
> +{

Can you add a "how to use" comment at the top? Also, does this need a copyright
notice?

> Source/WebCore/WebCore.gyp/WebCoreGTK.gyp:124
> +	       # The relative path here is to work around a quirk of gyp that
removes duplicate command-line arguments.

Maybe mention http://crbug.com/146682 in this comment.

> Source/WebCore/WebCore.gyp/WebCoreGTK.gyp:166
> +	       '<(WebCore)/inspector/xxd.pl',

Is it worth having a gypi file for xxd.pl actions too? It's used in a few
places (and slightly inconsistently: The last two uses don't list xxd.pl as an
input, the remaining do.)

> Source/WebCore/WebCore.gyp/WebCoreGTK.gyp:569
> +	       '<(WebCore)/dom/make_event_factory.pl',

Is this right? Does the python script shell out to the pl script? Or is this a
copy-pasto?

> Source/WebCore/WebCore.gyp/WebCoreGTK.gyp:588
> +	       '<(WebCore)/dom/make_event_factory.pl',

(same question)

> Source/WebCore/WebCore.gyp/WebCoreGTK.gyp:606
> +	       '<(WebCore)/dom/make_dom_exceptions.pl',

(same question. also in at least one more spot below, so I'll stop saying
this.)

> Source/WebCore/WebCore.gypi:-5774
> -	       '<(PRODUCT_DIR)/DerivedSources/WebCore/CSSGrammar.cpp',

Isn't this file used by chromium too? Won't this change break the chromium
build?