[webkit-reviews] review granted: [Bug 109360] [GTK] Connect the gyp build to autoconf : [Attachment 187906] Use gyp dependencies again and fix an issue with re-running gyp

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 109360: [GTK] Connect the gyp build to autoconf
https://bugs.webkit.org/show_bug.cgi?id=109360

Attachment 187906: Use gyp dependencies again and fix an issue with re-running
gyp
https://bugs.webkit.org/attachment.cgi?id=187906&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=187906&action=review


looks fine. I'll r+ this for now since no one commented on the autotools stuff
but I doubt it'll break anything so we can always fix it down the road.

> Source/WebKit/gtk/gyp/Configuration.gypi.in:22
> +    ],

If you are going to want these defined in every (C/C++) target, you might
consider using a 'target_defaults' block here so that you don't have to repeat
yourself all over the place. See an example in Chromium's
src/build/common.gypi.

> Source/WebKit/gtk/gyp/Configuration.gypi.in:26
> +    # '.' to the directory containing the gyp file.

"the" gyp file? which gyp file? The top-level one, as passed to run-gyp, or
*every* gyp file? I think it's actually the latter, but I'm not sure. At any
rate, the comment should be more explicit, since this is one of the less-clear
aspects of gyp. In the current patch, all of your gyp files are in the same
directory, but that'll likely change as you get more of the project working.