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: gradle indications were outdated #241

Merged
merged 10 commits into from
Jan 18, 2019
Merged

Conversation

cutiko
Copy link
Contributor

@cutiko cutiko commented Jan 18, 2019

replace compile for implementation

replace compile for implementation
Copy link
Collaborator

@brettchabot brettchabot left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@@ -11,8 +11,8 @@ Consider using the CountingIdlingResource class from the espresso-contrib packag
Note that the `espresso-idling-resource` dependency is added into the `compile` scope:
Copy link
Collaborator

Choose a reason for hiding this comment

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

change this to 'implementation' scope too ?

@@ -11,8 +11,8 @@ Consider using the CountingIdlingResource class from the espresso-contrib packag
Note that the `espresso-idling-resource` dependency is added into the `compile` scope:

```
androidTestCompile 'com.android.support.test.espresso:espresso-core:2.2.2'
compile 'com.android.support.test.espresso:espresso-idling-resource:2.2.2'
androidTestImplementation 'com.android.support.test.espresso:espresso-core:2.2.2'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the maven artifact references are also out of date. Would you mind updating the references to

androidTestImplementation 'androidx.test.espresso:espresso-core:3.1.1'
implementation 'androidx.test.espresso:espresso-idling-resource:3.1.1'

@cutiko
Copy link
Contributor Author

cutiko commented Jan 18, 2019

Greetings

Have updated the Espresso version on doc and gradle. Didn't want to do it with other dependencies because it seems out of the scope.

Took the liberty to add the difference between AndroidX and previous.

The other compile found on the readme was also changed.

Double checked by running the app and the test and both were fine.

This is my first `Changes requested', the instruction says I have to push to master, have done it. Do you need me to do something else?

@@ -44,5 +44,5 @@ ext {
coreVersion = "1.1.0-beta01"
extJUnitVersion = "1.1.0-beta01"
runnerVersion = "1.1.1-beta01"
Copy link
Collaborator

Choose a reason for hiding this comment

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

odd. these versions should all be stable, not beta01. Is this branch synced with latest master?

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 assumed master would be the latest, and since the change shouldn't be breaking, just keep my self on master as well.

I feel an example shouldn't have not stable libraries cause it could confuse developer in their learning process.

I'm gonna take look at what you point and will see if I can change the other dependencies while on it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops, in my local master branch these are stable. I'll look into it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please merge in master, I've merged a PR that updated versions to stable

@brettchabot brettchabot merged commit 383bcbe into android:master Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants