.
Created attachment 262784 [details] Proposed Fix
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.
(In reply to comment #2) > [...] > This duplicates code in function osXVersion. I meant to write: This duplicates code in function determineOSXVersion().
Comment on attachment 262784 [details] Proposed Fix Also, we should add iosVersion() to the list of exported functions @EXPORT.
(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.
Committed r190821: <http://trac.webkit.org/changeset/190821>