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-
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
Note You need to log in before you can comment on or make changes to this bug.