-
Notifications
You must be signed in to change notification settings - Fork 451
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
fix imports for ksp plugin #2678
Conversation
val sourceClassName = el.qualifiedName?.asString() ?: el.simpleName | ||
val sourceName = el.simpleName.asString().replaceFirstChar { it.lowercase(Locale.getDefault()) } | ||
val simpleName = el.simpleName.asString() | ||
val packageName = pckg.asString() |
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.
Can we keep these names the same? This is our own domain.
This introduces a lot of unnecessary changes in the PR.
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.
Sure
...-libs/optics/arrow-optics-ksp-plugin/src/main/kotlin/arrow/optics/plugin/internals/lenses.kt
Show resolved
Hide resolved
@@ -38,7 +38,6 @@ class LensTests { | |||
fun `Lenses which mentions imported elements`() { | |||
""" | |||
|$imports | |||
|import kotlin.time.Duration |
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.
These tests were not running if this bug was not caught. Is it possible to make them run?
If this import is removed, then it should be using FQN on line 47.
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.
Was removed since I added a variable in the dslModel with a Duration type so the import is automatically added in $import
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.
Let me change those to FQNames 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.
Looks great @i-walker! Thanks for the work on the fix ❤️
resolves #2618
Status:
In progres