-
Notifications
You must be signed in to change notification settings - Fork 7
Move to Kotlin 1.9.23 and clean up build and source files #19
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
Conversation
faogustavo
left a comment
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.
Approved with a few comments. None of them are blockers.
| wasmJs { | ||
| browser() | ||
| nodejs() | ||
| binaries.executable() | ||
| } |
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 think we can simplify as in the example below. What do you think? AFAIK These configurations are only for the final output.
| wasmJs { | |
| browser() | |
| nodejs() | |
| binaries.executable() | |
| } | |
| wasmJs() |
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.
If we can remove, also remove on JS please.
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.
What do you mean when you say These configurations are only for the final output. ?
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.
Ktor is a library and it defines the targets
https://github.com/ktorio/ktor/blob/6e52d76177a823b3c667989483e9bce35b04f53c/buildSrc/src/main/kotlin/TargetsConfig.kt#L45
No description provided.