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

 ##########
 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:
   For informational purposes, since Gradle scripts are written in Groovy, 
using double quotations gives you string interpolation. String literals works 👍 
   
   These three lines could be written in one as,
   
   ```
   "No usable Android build tools found. Highest installed version is 
${highestBuildToolsVersion.getOriginalString()}; minimum version required is 
${minBuildToolsVersionString}."
   ```
   
   If this way is preferred just because the string is long, I would suggest 
single quotes so there is no string interpolation.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to