WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
42911
Update ruby tools to work with shallow framework bundles
https://bugs.webkit.org/show_bug.cgi?id=42911
Summary
Update ruby tools to work with shallow framework bundles
David Kilzer (:ddkilzer)
Reported
2010-07-23 14:27:02 PDT
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(-)
Attachments
Patch
(4.03 KB, patch)
2010-07-23 14:27 PDT
,
David Kilzer (:ddkilzer)
mrowe
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Mark Rowe (bdash)
Comment 1
2010-07-23 14:41:07 PDT
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!
David Kilzer (:ddkilzer)
Comment 2
2010-07-23 14:59:58 PDT
Committed
r63997
: <
http://trac.webkit.org/changeset/63997
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug