[webkit-reviews] review granted: [Bug 220080] [Cocoa] Add a script to generate the "Feature Flags" plist file : [Attachment 416652] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 220080: [Cocoa] Add a script to generate the "Feature Flags" plist file
https://bugs.webkit.org/show_bug.cgi?id=220080

Attachment 416652: Patch

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




--- Comment #5 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 416652
  --> https://bugs.webkit.org/attachment.cgi?id=416652
Patch

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

> Source/WebKit/FeatureFlags/WebKit-ios.plist:60
> +    <key>WebGL2</key>

Ok as is, this key and 2 below don't seem to have same formatting.

> Source/WebKit/Scripts/generate-feature-flags-plist.sh:26
> +if [[ ${WK_PLATFORM_NAME} == "appletvos" ]]; then

Does this work for simulator, too? Same question apples to ios and watch.

> Source/WebKit/Scripts/generate-feature-flags-plist.sh:35
> +    exit 0;

This seems like a programmer error to reach. If so. consider emitting error
message and exiting with non-zero return code. Error message should conform to
special Xcode syntax to get nice GUI results:

error: Message.

 See
<https://stackoverflow.com/questions/7239312/can-i-fully-customise-an-xcode-4-r
un-script-build-phase-error-warning-within-the> Etc etc.

> Source/WebKit/Scripts/generate-feature-flags-plist.sh:37
> +

Ok as is, consider adding "set -e" so that command failure below causes script
to exit immediately.

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:13099
> +			       
"$(INSTALL_ROOT)/$(WK_INSTALL_PATH_PREFIX)/$(SYSTEM_LIBRARY_DIR)/FeatureFlags/D
omain/WebKit.plist",

Ok as is. Is this necessary?