Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use gmtool after configuration has been done #20

Conversation

sgaland
Copy link
Contributor

@sgaland sgaland commented Feb 2, 2017

In order to be able to use the plugin with configuration in the local.properties and without putting gmtool in the path, move some codes to be sure gmtool has been configured before using it to retrieve its version.

It works on my side for my use case, please review this to be sure there is no side effect I missed.

@@ -87,7 +87,7 @@ dependencies {
}

group = 'com.genymotion'
version = '1.3'
version = '1.4'
Copy link
Contributor

Choose a reason for hiding this comment

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

please don't bump the version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oups.

gmtool.setConfig(config)

project.genymotion.config.version = gmtool.getVersion()
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be outside the if (line 250) as it is needed whatever the config is set or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright

In order to be able to use the plugin with configuration in the
local.properties and without putting gmtool in the path, move some
codes to be sure gmtool has been configured before using it to retrieve
its version.
@sgaland sgaland force-pushed the hotfix/gmtool_used_before_configuration branch from 8decfe3 to c61af1c Compare February 2, 2017 23:42
@eyal-lezmy
Copy link
Contributor

Thanks for the modification. We need to find a way to test this bug efficiently before merging. I'll try to look at it next week but feel free to propose if you have time.

@eyal-lezmy eyal-lezmy changed the base branch from master to integration/fix-gmtool-config August 4, 2017 12:00
Copy link
Contributor

@eyal-lezmy eyal-lezmy left a comment

Choose a reason for hiding this comment

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

Let's merge this on an integration branch and add unit tests!

@eyal-lezmy eyal-lezmy merged commit a8f0b37 into Genymobile:integration/fix-gmtool-config Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants