Move to Kotlin #76
Move to Kotlin #76
Conversation
erwinw
requested review from
andriygg,
EliasJorgensen and
andersfischernielsen
| # v2.7.0 is giving issues | ||
| uses: cycjimmy/semantic-release-action@v2.6.0 |
There was a problem hiding this comment.
What issues? I'm using it without issue in jwt-kt 🤔
There was a problem hiding this comment.
As you can see in this run, it fails without any useful info.
There was a problem hiding this comment.
Initial quick look looks good to me - this is quite a big one to review, so I'm trusting you semi-blindly here @erwinw ❤️🔥
prop-archaius/src/main/kotlin/io/pleo/prop/archaius/ParsingProperty.kt
Outdated
Show resolved
Hide resolved
| // Static properties can't change. No callbacks. | ||
| override fun addCallback(callback: Runnable) {} | ||
| override fun removeAllCallbacks() {} |
There was a problem hiding this comment.
Can we add this comment so it's visible in engineers' editor? Unsure if this is JavaDoc or how IntelliJ pulls out that information - WDYT? Could limit engineers being unpleasantly surprised
erwinw
force-pushed
the
chore/move-to-kotlin
branch
from
457e8c2
to
8f31466
Compare
Trust is good, testing is better -- I've tested this locally by using it with JVM Commons and building & testing it. |
| } | ||
|
|
||
| repositories { | ||
| mavenLocal() |
There was a problem hiding this comment.
it worth to include only jvm-commons here, like:
mavenLocal {
content {
// this repository *only* contains artifacts with group "pleo-io.jvm-commons"
includeGroup "pleo-io.jvm-commons"
}
}
There was a problem hiding this comment.
Eh no; Prop is a public repo and does not rely on JVM Commons!
| val parser = parserFactory.createParserForType(type) | ||
| try { | ||
| val annotation = parameter.getAnnotation(Default::class.java) | ||
| println("ANNOTATION: $annotation") |
There was a problem hiding this comment.
this guy should be removed I think
| val parameter = parameters[dependency.parameterIndex] | ||
| PropResult(parameterToProp(parameter, key)) | ||
| } catch (ex: RuntimeException) { | ||
| PropResult(ex) |
There was a problem hiding this comment.
interesting, how does it work if PropResult will be an exception?
There was a problem hiding this comment.
In a follow-up PR I'm replacing this with the more idiomatic Kotlin Result.
| .map(Default::value) | ||
| .map(parser::apply) | ||
| .orElse(null) | ||
| println("DEF: $defaultValue") |
|
|
||
| private fun getNamedAnnotationValue(annotations: List<Annotation>, key: Key<*>): String = | ||
| annotations.mapNotNull(::annotationValueIfNamed).lastOrNull().also { | ||
| println("?? $it") |
There was a problem hiding this comment.
one more print to be either removed or logger used
| is com.google.inject.name.Named -> annotation.value | ||
| else -> null | ||
| }?.ifEmpty { null }.also { | ||
| println(">> $it") |
| this.error = error | ||
| } | ||
|
|
||
| fun isError() = error != null |
There was a problem hiding this comment.
OK, now I see how it works! 👍
|
🎉 This PR is included in version 4.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Move to Kotlin
Move the existing Java implementation to Kotlin
Breaking changes: any breaking changes EXCEPT one: fixing the package for
JacksonParserFactory(it was mistakenly put as.guiceinstead of.jackson).