Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Inherit the SDK path from the android plugin #70

Merged
merged 5 commits into from
Feb 20, 2015

Conversation

emanuelez
Copy link
Contributor

The android grade plugin already determines the SDK path and exposes it so it is not needed to duplicate such logic. Unfortunately it is done in different ways in different versions of the android plugin.

The unit test about androidHome is now useless, so it has been removed.

…ch logic.

The android grade plugin already determines the SDK path and exposes it.
Unfortunately it is done in different ways in different versions.

The unit test about androidHome is now useless, so it has been removed.
@emanuelez
Copy link
Contributor Author

This has been triggered because when the plugin fails to find the SDK in local.properties or ANDROID_HOME it would be hard for a user to figure out what went wrong. Only examining the stacktrace of the failed Gradle build would help.

@@ -11,7 +11,16 @@ public class AndroidCommandPlugin implements Plugin<Project> {
if (!project.plugins.hasPlugin('android')) {
throw new StopExecutionException("The 'android' plugin is required.")
}
def extension = project.android.extensions.create("command", AndroidCommandPluginExtension, project)

def android = project.android
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the name confusing.

throw new IllegalStateException("Couldn't read the SDK directory. If you're running the tests, " +
"make sure you set the ANDROID_HOME env. variable and it points to your Android SDK home. Otherwise, " +
"make sure there's a local.properties in the root of your project with the property sdk.dir pointing to the Android SDK")
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to keep some checking here, in case the android plugin API changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I introduced a check in the SDK detection in order to fail even earlier in case the android plugin changes API again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@devisnik
Copy link
Contributor

Looks good, thanks for contributing!
Some minor fixes and this should be good to merge.

Also reintroduce the androidHome unit test
@xrigau
Copy link
Contributor

xrigau commented Feb 19, 2015

👍

} else if(androidExtension.hasProperty('sdkDirectory')) {
androidHome = "${androidExtension.sdkDirectory}"
} else {
throw new IllegalStateException('The android plugin is not exposing the SDK folder in an expected way.')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

devisnik added a commit that referenced this pull request Feb 20, 2015
Inherit the SDK path from the android plugin
@devisnik devisnik merged commit 17c4adf into novoda:develop Feb 20, 2015
@devisnik
Copy link
Contributor

Thanks for contributing, @emanuelez !

@emanuelez emanuelez deleted the inherit-sdk-path branch February 20, 2015 17:57
@emanuelez
Copy link
Contributor Author

My pleasure! Are you guys planning a release by any chance? :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants