[webkit-reviews] review denied: [Bug 238409] [XCBuild] WebKitLegacy's "Migrated headers" script does not emit task information : [Attachment 455924] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Elliott Williams
<emw at apple.com>'s request for review:
Bug 238409: [XCBuild] WebKitLegacy's "Migrated headers" script does not emit
task information
https://bugs.webkit.org/show_bug.cgi?id=238409

Attachment 455924: Patch

https://bugs.webkit.org/attachment.cgi?id=455924&action=review




--- Comment #4 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 455924
  --> https://bugs.webkit.org/attachment.cgi?id=455924
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455924&action=review

Marking r- because this breaks the build.

> Source/WebKitLegacy/ChangeLog:25
> +	   multicore machine it should be faster than blocking WebKitLegacy's
build process on the

This matches my instincts, yet I'm curious if you measured the difference.

> Source/WebKitLegacy/mac/ChangeLog:11
> +	   USE_RECURSIVE_SCRIPT_INPUTS_IN_SCRIPT_PHASES, so that the "Generate
Export Files" phase can

I didn't know this existed!

> Source/WebKitLegacy/mac/Configurations/WebKitLegacy.xcconfig:38
> +// WebEventRegion.h is migrated when building for an iOS family target with
an internal SDKâ¦
> +INCLUDED_MIGRATED_HEADERS_YES_cocoatouch = WebEventRegion.h;
> +// â¦or when --ios-touch-events was passed to build-webkit manually.

I'm guessing these are ellipses? Not sure how I feel about them, would probably
use ASCII.

> Source/WebKitLegacy/mac/Configurations/WebKitLegacy.xcconfig:42
> +// The "Generate Export Files" build pahse needs this setting to rerun when
any migrated header changes:

typo: pahse.

> Tools/ChangeLog:11
> +	   script should check PrivateHeaders/ too.

Not sure if this comment matches what the change does. The change switches from
"Headers" to "PrivateHeaders", so it's not obvious why it's a "too".