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

Bump OkHttp #1094

Merged
merged 4 commits into from Jul 9, 2019

Conversation

Projects
None yet
3 participants
@jjestrel
Copy link
Collaborator

commented Jul 9, 2019

Update to OkHttp4

Auto converted OkHttp changes are in this commit:
0e35894

Non-backwards compatible changes are handled in this commit:
7ad5c68

Test updates
17f3375

Jay Estrella added some commits Jul 9, 2019

@jjestrel

This comment has been minimized.

This should be in the first commit but I missed it

@jjestrel jjestrel requested review from swankjesse and nbroyles Jul 9, 2019

@adrw

adrw approved these changes Jul 9, 2019

Copy link
Collaborator

left a comment

So much more Kotlin-y!

Jay Estrella
@@ -74,7 +75,7 @@ internal class ResourceExhaustedExceptionMapper @Inject internal constructor() :

internal object ConflictExceptionResponder {
private val HEADERS: Headers =
Headers.of(listOf("Content-Type" to MediaTypes.TEXT_PLAIN_UTF8).toMap())
listOf("Content-Type" to MediaTypes.TEXT_PLAIN_UTF8).toMap().toHeaders()

This comment has been minimized.

Copy link
@swankjesse

swankjesse Jul 9, 2019

Collaborator

could follow-up to do just headersOf("Content-Type", MediaTypes.TEXT_PLAIN_UTF8)

This comment has been minimized.

Copy link
@jjestrel

jjestrel Jul 9, 2019

Author Collaborator

changed

}
appName == ou
}
.hostnameVerifier(object : HostnameVerifier {

This comment has been minimized.

Copy link
@swankjesse

swankjesse Jul 9, 2019

Collaborator

this is the most annoying part of the Kotlin upgrade

@@ -31,7 +31,9 @@ internal class QueryParamFeatureBinding private constructor(
private val name: String
) {
fun bind(subject: Subject) {
val values = subject.httpCall.url.queryParameterValues(name)
val values = subject.httpCall.url.queryParameterValues(name).map {
it ?: throw IllegalArgumentException()

This comment has been minimized.

Copy link
@swankjesse

swankjesse Jul 9, 2019

Collaborator

this is correct but I think we later want to make a decision on what we do for b in https://host/path?a=1&b&c=2

Show resolved Hide resolved misk/src/test/kotlin/misk/client/MockWebServerService.kt Outdated
@jjestrel

This comment has been minimized.

Copy link
Collaborator

commented on misk/src/main/kotlin/misk/client/PeerClientFactory.kt in 7ad5c68 Jul 9, 2019

changing to ?. instead of !!

Jay Estrella

@jjestrel jjestrel force-pushed the jayestrella/20190709-bump-okhttp branch from 643ce9d to fd0b004 Jul 9, 2019

@jjestrel jjestrel merged commit b020274 into master Jul 9, 2019

2 checks passed

ci/circleci: java Your tests passed on CircleCI!
Details
ci/circleci: node Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.