[GitHub] [cordova-android] raphinesse commented on a change in pull request #900: refactor: simplify doFindLatestInstalledBuildTools

[GitBox <gi...@apache.org>]

raphinesse commented on a change in pull request #900: refactor: simplify doFindLatestInstalledBuildTools
URL: https://github.com/apache/cordova-android/pull/900#discussion_r367866071
 
 

 ##########
 File path: framework/cordova.gradle
 ##########
 @@ -40,40 +40,40 @@ String doGetProjectTarget() {
     return doEnsureValueExists('project.properties', props, 'target')
 }
 
-Version[] getAvailableBuildTools() {
-    def buildToolsDir = new File(getAndroidSdkDir(), "build-tools")
-    buildToolsDir.list()
-        .collect { new Version(it) } // Invalid inputs will be handled as 0.0.0
-        .findAll { it.isHigherThan('0.0.0') }
-        .sort { a, b -> a.isHigherThan(b) ? -1 : 1 }
-}
-
 Boolean isVersionValid(version) {
     return !(new Version(version)).isEqual('0.0.0')
 }
 
 String doFindLatestInstalledBuildTools(String minBuildToolsVersionString) {
-    def availableBuildToolsVersions
+    def buildToolsDirContents
     try {
-        availableBuildToolsVersions = getAvailableBuildTools()
+        def buildToolsDir = new File(getAndroidSdkDir(), "build-tools")
+        buildToolsDirContents = buildToolsDir.list()
     } catch (e) {
         println "An exception occurred while trying to find the Android build tools."
         throw e
     }
-    if (availableBuildToolsVersions.length > 0) {
-        def highestBuildToolsVersion = availableBuildToolsVersions[0]
-        if (highestBuildToolsVersion.isLowerThan(minBuildToolsVersionString)) {
-            throw new RuntimeException(
-                "No usable Android build tools found. Highest installed version is " +
-                highestBuildToolsVersion.getOriginalString() + "; minimum version required is " +
-                minBuildToolsVersionString + ".")
-        }
-        highestBuildToolsVersion.getOriginalString()
-    } else {
+
+    def highestBuildToolsVersion = buildToolsDirContents
+        .collect { new Version(it) }
+        // Invalid inputs will be handled as 0.0.0
+        .findAll { it.isHigherThan('0.0.0') }
+        .max()
+
+    if (highestBuildToolsVersion == null) {
         throw new RuntimeException(
             "No installed build tools found. Install the Android build tools version " +
             minBuildToolsVersionString + " or higher.")
     }
+
+    if (highestBuildToolsVersion.isLowerThan(minBuildToolsVersionString)) {
+        throw new RuntimeException(
+            "No usable Android build tools found. Highest installed version is " +
+            highestBuildToolsVersion.getOriginalString() + "; minimum version required is " +
+            minBuildToolsVersionString + ".")
 
 Review comment:
   I pushed a commit that uses string interpolation for the error messages while still avoiding line lengths of > 80 characters. What do you think of it?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org