WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
149963
Teach webkitperl how to figure out IOS versions from the SDK
https://bugs.webkit.org/show_bug.cgi?id=149963
Summary
Teach webkitperl how to figure out IOS versions from the SDK
Blaze Burg
Reported
2015-10-09 11:55:58 PDT
.
Attachments
Proposed Fix
(2.79 KB, patch)
2015-10-09 12:02 PDT
,
Blaze Burg
dbates
: review+
dbates
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Blaze Burg
Comment 1
2015-10-09 12:02:17 PDT
Created
attachment 262784
[details]
Proposed Fix
Daniel Bates
Comment 2
2015-10-09 12:09:27 PDT
Comment on
attachment 262784
[details]
Proposed Fix View in context:
https://bugs.webkit.org/attachment.cgi?id=262784&action=review
> Tools/Scripts/copy-webkitlibraries-to-product-directory:235 > + my $majorSDKVersion = IOSVersion()->{"major"};
We tend to follow the Webkit Code Style Guidelines for Perl code. So, this should be named iosVersion().
> Tools/Scripts/webkitdirs.pm:118 > +my $IOSVersion;
Similarly this should be name iosVersion.
> Tools/Scripts/webkitdirs.pm:1294 > + $IOSVersion = -1;
Ditto.
> Tools/Scripts/webkitdirs.pm:1301 > + $IOSVersion = {
Ditto.
> Tools/Scripts/webkitdirs.pm:1306 > + my $version = xcodeSDKVersion(); > + my @splitVersion = split(/\./, $version); > + @splitVersion >= 2 or die "Invalid version $version"; > + $IOSVersion = { > + "major" => $splitVersion[0], > + "minor" => $splitVersion[1], > + "subminor" => (defined($splitVersion[2]) ? $splitVersion[2] : 0), > + }; > +}
This duplicates code in function osXVersion. Would it make sense to extract the common code into a shared function, say splitVersionString(), and call it from both this function and determineOSXVersion()?
> Tools/Scripts/webkitdirs.pm:1308 > +sub IOSVersion()
This should be named iosVersion.
> Tools/Scripts/webkitdirs.pm:1311 > + return $IOSVersion;
This should be named iosVersion.
Daniel Bates
Comment 3
2015-10-09 12:10:36 PDT
(In reply to
comment #2
)
> [...] > This duplicates code in function osXVersion.
I meant to write: This duplicates code in function determineOSXVersion().
Daniel Bates
Comment 4
2015-10-09 12:15:28 PDT
Comment on
attachment 262784
[details]
Proposed Fix Also, we should add iosVersion() to the list of exported functions @EXPORT.
Blaze Burg
Comment 5
2015-10-09 12:23:17 PDT
(In reply to
comment #2
)
> Comment on
attachment 262784
[details]
> > Tools/Scripts/webkitdirs.pm:1306 > > + my $version = xcodeSDKVersion(); > > + my @splitVersion = split(/\./, $version); > > + @splitVersion >= 2 or die "Invalid version $version"; > > + $IOSVersion = { > > + "major" => $splitVersion[0], > > + "minor" => $splitVersion[1], > > + "subminor" => (defined($splitVersion[2]) ? $splitVersion[2] : 0), > > + }; > > +} > > This duplicates code in function osXVersion. Would it make sense to extract > the common code into a shared function, say splitVersionString(), and call > it from both this function and determineOSXVersion()?
I could, though I'm not sure that subminors are even used by iOS SDKs.
Blaze Burg
Comment 6
2015-10-09 15:01:47 PDT
Committed
r190821
: <
http://trac.webkit.org/changeset/190821
>
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