-
Notifications
You must be signed in to change notification settings - Fork 28
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
Depend upon Kotlin 1.5 #348
Conversation
…ethods. Minor cleanup in xml serde.
@@ -93,7 +93,7 @@ internal fun XmlToken?.isTerminal(minimumDepth: Int = 0) = when (this) { | |||
else -> depth < minimumDepth | |||
} | |||
|
|||
internal fun XmlToken?.isNotTerminal(minimumDepth: Int = 0) = !this.isTerminal() | |||
internal fun XmlToken?.isNotTerminal(minimumDepth: Int = 0) = !this.isTerminal(minimumDepth) |
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 was a bug from me
override suspend fun call(request: String): String = request.capitalize() | ||
override suspend fun call(request: String): String = request.replaceFirstChar { c -> c.uppercaseChar() } |
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.
Question: I kinda like the old capitalize()
and decapitalize()
semantics. Is there a principled reason not to add new extension methods that replace the deprecated ones?
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.
It would def be more concise. Unsure of how principled this is but we'd want to name them something else as to disambiguate from the deprecated stdlib functions to avoid confusion. I couldn't come up w/ a name I thought would pass the naming trial so opted to just go w/ the advice from the compiler.
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 too prefer the shorter versions but have no better names to offer (also I can live with the changes as is if that's where we land).
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 spent a few mins trying to come up w/ a name but nada. Will go w/ what is here for now. It's a quick search/replace when/if we come up w/ something better that doesn't conflict w/ stdlib.
override suspend fun call(request: String): String = request.capitalize() | ||
override suspend fun call(request: String): String = request.replaceFirstChar { c -> c.uppercaseChar() } |
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 too prefer the shorter versions but have no better names to offer (also I can live with the changes as is if that's where we land).
Issue #, if available: #319
This PR is in draft form until important Kotlin lib dependencies we use are on 1.5x (currently this is the coroutines dep still in RC status for 1.5)
Description of changes:
Bump version of Kotlin compiling runtime and services against to resolve runtime classpath issue that is hit when depending on 1.4x built version of SDK (see issue for more details)
Testing Done
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.