[webkit-reviews] review denied: [Bug 105955] [Chromium] Add Platform/chromium to include_dirs : [Attachment 181077] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Mark Pilgrim (Google)
<pilgrim at chromium.org>'s request for review:
Bug 105955: [Chromium] Add Platform/chromium to include_dirs
https://bugs.webkit.org/show_bug.cgi?id=105955

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=181077&action=review


Close!

> Source/WebKit/chromium/WebKit.gyp:702
> +				   'include_dirs': [
> +				       '../../Platform/chromium/',
> +				   ],

I don't think you need to add this in the 'component=="shared_library"'
conditional since it applies equally to component and non-component builds.

> Source/WebKit/chromium/WebKit.gyp:777
> +			       '../../Platform/chromium/',

similarly, this include path doesn't depend on the value of
'inside_chromium_build' so it shouldn't be inside a conditional block. could
you add a new 'direct_dependent_settings' block outside the conditionals?