Skip to content
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

chore: simplify KotlinCompile configuration #1343

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

patrick-dedication
Copy link
Contributor

Reason: the kotlin-gradle-plugin respects JvmToolchains, see here.

In case you wanted to be explicit about it, feel free to just close this PR :)

Copy link
Owner

@autonomousapps autonomousapps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't used to always respect it!

@patrick-dedication
Copy link
Contributor Author

So merge or close?

@autonomousapps
Copy link
Owner

I like simplifying things, but what I want before merging is some way to validate that the artifacts before and after the change are the same -- that they're built with the expected JVM target.

Reason: the kotlin-gradle-plugin respects JvmToolchains
@patrick-dedication patrick-dedication force-pushed the simplify-kotlin-compile-configuration branch from 09a71ee to ccb8c9d Compare December 21, 2024 06:16
@patrick-dedication
Copy link
Contributor Author

Do you an automated test?
Or once with javap -verbose -classpath build-logic/convention/build/libs/convention.jar com.autonomousapps.convention.ConventionPlugin | grep -b20 -a20 "major version"?

A thought that came to me a few days ago:
This configuration configures the bytecode version for the plugins.
As far as I understand it the convention plugins runs in the gradle jvm.
I think using toolchains it is possible to compile them to a different version than gradle is run with.
So is it even wise to use toolchains at all in this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants