-
Notifications
You must be signed in to change notification settings - Fork 2
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
Upgrade to Android Gradle Plugin 4.2.0 #1
Conversation
randallnwf
commented
Mar 16, 2022
- Upgrade to Gradle 6.9
- Upgrade incompatible libraries
- Fixes in okreplay-sample
- Convert some Spock (Groovy) unit tests to Kotlin in okreplay-tests
* Upgrade to Gradle 6.9 * Upgrade incompatible libraries * Fixes in okreplay-sample * Convert some Spock (Groovy) unit tests to Kotlin in okreplay-tests
val request: Request = Request.Builder().url(endpoint.url("/")).build() | ||
val response = client.newCall(request).execute() | ||
Assert.assertEquals(HttpURLConnection.HTTP_OK.toLong(), response.code.toLong()) | ||
Assert.assertEquals("OkReplay", response.header(Util.VIA)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the test strings in this class could be extracted to a companion object as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well ... I don't think we're going to preserve any history with the Spock test.
implementation 'com.squareup.retrofit2:converter-moshi:2.3.0' | ||
implementation 'com.squareup.retrofit2:adapter-rxjava2:2.3.0' | ||
implementation 'com.squareup.retrofit2:converter-moshi:2.3.0' | ||
implementation "com.google.android.material:material:1.5.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just demo-related UI?
Material sometimes does breaking design changes in minor releases, so we'd have to visit the page to make sure it still looks right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is just a demo app, and it does have a couple UI tests.
okreplay-gradle-plugin/build.gradle
Outdated
validateTaskProperties { | ||
failOnWarning = true | ||
} | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to be reenabled at a later time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good eye! I disabled that when I went for broke trying to upgrade to AGP 7.x. Now I can kick that can!
@@ -113,7 +115,7 @@ class OkReplayInterceptor : Interceptor { | |||
} | |||
|
|||
private fun isHostIgnored(request: okhttp3.Request): Boolean { | |||
return configuration!!.ignoreHosts.contains(request.url().host()) | |||
return configuration!!.ignoreHosts.contains(request.url.host) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an unsafe access in non-demo code. Could it be changed to a safe access instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I see that this is a problem in a bunch of places. This is optional!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll save the qualitative changes for smaller improvement-related PRs :)
classpath dep.gradleMavenPublishPlugin | ||
} | ||
} | ||
|
||
allprojects { | ||
repositories { | ||
jcenter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
Upgrade to Gradle 7.3.3 Update JVM target to 11 Added dependencies that have been moved