Summary: | Update ruby tools to work with shallow framework bundles | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Component: | Tools / Tests | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | mrowe | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Comment on attachment 62460 [details]
Patch
A few minor comments, primarily stylistic
I think that:
has_shallow_bundle = (ENV['SHALLOW_BUNDLE'] || "NO").upcase == “YES”
would be clearer than:
has_shallow_bundle = !ENV['SHALLOW_BUNDLE'].nil? && ENV['SHALLOW_BUNDLE'].upcase == “YES”
You’re using has_shallow_bundle and is_shallow to mean the same thing. I’d pick one name and stick with it. I think “is_shallow_bundle” would be a good compromise.
49 all_headers = is_shallow ? `find WebKit.framework/{,Private}Headers -type f -name '*.h'`.split \
50 : `find WebKit.framework/Versions/A/{,Private}Headers -type f -name '*.h'`.split
Duplicating the entire command here is unfortunate. I’d suggest something like:
current_version_path = is_shallow_bundle ? “” : “Versions/A/”
all_headers = `find WebKit.framework/#{current_version_path}{,Private}Headers -type f -name '*.h'`.split
(Excuse the curly quotes :-/)
Other than that, r=me!
Committed r63997: <http://trac.webkit.org/changeset/63997> |
Created attachment 62460 [details] Patch Reviewed by NOBODY (OOPS!). * Scripts/check-for-inappropriate-files-in-framework: Added check for the SHALLOW_BUNDLE environment variable so that the script will work with iOS WebKit builds. * Scripts/check-for-webkit-framework-include-consistency: Ditto. --- 3 files changed, 23 insertions(+), 8 deletions(-)