Implemented additional compiler flags for KT-75078 #531
Conversation
|
@shanshin hi! Could you please have a look, if my changes for compiler flags make sense? |
| fun KotlinCommonCompilerOptions.addExtraCompilerFlags(project: Project) { | ||
| val extraOptions = project.rootProject.properties["kotlin_additional_cli_options"] as? String | ||
| if (extraOptions != null) { | ||
| LOGGER.info("""Adding extra compiler flags '$extraOptions' for a compilation in the project $${project.name}""") |
There was a problem hiding this comment.
It makes sense to use the same logging format across all our projects, and the one from kotlinx-serialization looks fine: https://github.com/Kotlin/kotlinx.serialization/pull/2996/files#diff-09e3a69c8143d7af22e62f518f556102bbdea00485f9e30be972303bc6c7dc19R58
There was a problem hiding this comment.
Thank you) I've fixed that
| * `true` means that warnings should be treated as errors,`false` means that they should not. | ||
| */ | ||
| private fun warningsAreErrorsOverride(project: Project): Boolean? = | ||
| when (val prop = project.rootProject.properties["kotlin_Werror_override"] as? String) { |
There was a problem hiding this comment.
It's better to use providers.gradleProperty instead of project.rootProject.properties, see the changes in the serialization
| * Pass additional CLI options to the Kotlin compiler. | ||
| */ | ||
| fun KotlinCommonCompilerOptions.addExtraCompilerFlags(project: Project) { | ||
| val extraOptions = project.rootProject.properties["kotlin_additional_cli_options"] as? String |
There was a problem hiding this comment.
Same about project.rootProject.properties
|
@fzhinkin, hi!) Are we good to merge these changes? |
| */ | ||
| fun KotlinCommonCompilerOptions.addExtraCompilerFlags(project: Project) { | ||
| project.providers.gradleProperty("kotlin_additional_cli_options").orNull?.let { options -> | ||
| options.removeSurrounding("\"").split(" ").filter { it.isNotBlank() }.forEach { |
There was a problem hiding this comment.
We discussed it with @woainikk, and it seems like removeSurrounding("\"") is unnecessary here.
There was a problem hiding this comment.
Ok, I just saw that in the implementation for serialization and added here as well
There was a problem hiding this comment.
after implementing it in serialization, we understand that we can avoid it by passing parameters in other way, so yes, it's not needed anymore, sorry for confusing
No description provided.