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

Update to Android Studio 3.0 #302

Merged
merged 2 commits into from
Nov 6, 2017

Conversation

mapyo
Copy link
Contributor

@mapyo mapyo commented Oct 31, 2017

Thank you!

@mapyo mapyo changed the title Update Android Studio 3.0 Update to Android Studio 3.0 Oct 31, 2017
@mapyo mapyo force-pushed the update-android-studio-3.0 branch 2 times, most recently from 65d4968 to 202c750 Compare October 31, 2017 23:24
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-3.5-all.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

use ./gradlew wrapper --gradle-version 4.1 --distribution-type all to upgrade gradle. You did not update the gradle-wrapper.jar

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-3.5-all.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

4.3 is available, why have you picked 4.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed the latest version. to correct.

Copy link
Contributor

@passsy passsy left a comment

Choose a reason for hiding this comment

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

upgrade gradle wrapper correctly

@piotrek1543
Copy link
Contributor

@passsy what about changing buildToolsVersion to 27.0.0.

Support library version is not updated in that pull request, it's 25.3.1.

@uKL
Copy link
Collaborator

uKL commented Nov 2, 2017

I'm ok with changing buildToolsVersion to 27.0.0 in the same PR. Good idea.

@@ -1,6 +1,6 @@
ext {
supportVersion = '25.3.1'
buildToolsVersionVariable = "25.0.3"
buildToolsVersionVariable = "27.0.0"
Copy link

@friederbluemle friederbluemle Nov 2, 2017

Choose a reason for hiding this comment

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

You can remove this line, and all three occurrences of buildToolsVersion from the module's build.gradle files. Android Gradle plugin 3.0.0 will automatically pick the right build tools version.

Copy link
Contributor

@piotrek1543 piotrek1543 Nov 2, 2017

Choose a reason for hiding this comment

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

@friederbluemle well, I haven't noticed this... I definitely need to give a try this solution :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I haven't notice this too. Thank you.

https://developer.android.com/studio/releases/build-tools.html

If you're using Android plugin for Gradle 3.0.0 or higher, your project automatically uses a default version of the build tools that the plugin specifies.

I remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When deleting buildToolsVersion, 26.0.2 was used.
I think that it is better to use the latest version if possible. So I specify 27.0.0.

Choose a reason for hiding this comment

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

Interesting. I'm all for using the latest version if possible, but to be honest I think since Google implemented (and recommends to use) this auto-chooser, they know best which version to use for a particular Android Gradle plugin and compile/target SDK version combination.

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-3.5-all.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-4.3-all.zip

Choose a reason for hiding this comment

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

Looks good now, can you squash the commits please @mapyo?

Copy link
Owner

Choose a reason for hiding this comment

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

We will use Squash and merge option when merging

Copy link
Collaborator

@uKL uKL left a comment

Choose a reason for hiding this comment

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

It looks like there is some issue compiling Groovy code. Could you take a look?

@dariuszseweryn
Copy link
Owner

Thanks for the PR @mapyo and @friederbluemle @passsy @piotrek1543 for review!
I would like to have two things:

  1. Merge master to this branch so we could use Squash and merge when appropriate
  2. I agree with @uKL that tests should be working before merging this PR. Apparently the Support for Android Gradle Plugin Version 3.0.0 groovy/groovy-android-gradle-plugin#156 is blocking us
    We should either wait for a plugin in version 1.3.0 or to workaround the issue somehow?

jcenter()
}

dependencies {
classpath 'com.android.tools.build:gradle:2.3.3'
classpath 'com.android.tools.build:gradle:3.0.0'
classpath 'com.github.ben-manes:gradle-versions-plugin:0.14.0'
classpath 'org.codehaus.groovy:groovy-android-gradle-plugin:1.2.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

@friederbluemle if @dariuszseweryn is right, maybe temporary changing to classpath 'org.codehaus.groovy:groovy-android-gradle-plugin:1.3.0-SNAPSHOT' would fix groovy compilation errors

Copy link

@friederbluemle friederbluemle Nov 2, 2017

Choose a reason for hiding this comment

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

Good idea, you could definitely give it a try! Ultimately it's up to you and the maintainers of RxAndroidBle if a snapshot dependency on the master branch is acceptable.
If the update does fix the issue and this is the only problem, maybe we could nudge groovy-android-gradle-plugin to release an official new version soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried it locally. However, the test did not pass. I think that the change has not been uploaded in the snapshot yet.

http://oss.jfrog.org/oss-snapshot-local/org/codehaus/groovy/groovy-android-gradle-plugin/

1.3.0-SNAPSHOT/ 07-Jun-2017 17:25

@mapyo
Copy link
Contributor Author

mapyo commented Nov 5, 2017

Thanks for the review @dariuszseweryn @friederbluemle @passsy @piotrek1543 @uKL.

I agree with @uKL that tests should be working before merging this PR. Apparently the groovy/groovy-android-gradle-plugin#156 is blocking us
We should either wait for a plugin in version 1.3.0 or to workaround the issue somehow?

I committed a workaround to pass the test. Please confirm.

Copy link
Collaborator

@uKL uKL left a comment

Choose a reason for hiding this comment

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

I'm okay with the workaround, nice to have it working again. I'll create an issue so we'll replace the workaround with new plugin version when published.

@uKL uKL merged commit cc6e24e into dariuszseweryn:master Nov 6, 2017
@friederbluemle
Copy link

Nice 👍

@mapyo
Copy link
Contributor Author

mapyo commented Nov 6, 2017

Thank you 👍

@mapyo mapyo deleted the update-android-studio-3.0 branch November 7, 2017 01:21
@uKL
Copy link
Collaborator

uKL commented Mar 20, 2018

Dear Contributor,

similar to many open source projects we kindly request to sign our CLA if you'd like to contribute to our project. This license is for your protection as a Contributor as well as the protection of Polidea; it does not change your rights to use your own Contributions for any other purpose.

You can find a link here: https://cla-assistant.io/Polidea/RxAndroidBle

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.

6 participants