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

Downgrade codegen and remaining subprojects to JDK 17 #75

Merged
merged 2 commits into from
Apr 26, 2024

Conversation

hpmellema
Copy link
Contributor

Description of changes

Downgrades codegen to JDK 17.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@hpmellema hpmellema requested a review from adwsingh April 26, 2024 15:14
@rhernandez35
Copy link
Contributor

We can and should keep the code generator itself on 21. We only need to target producing 17 bytecode for the client shapes.

@hpmellema
Copy link
Contributor Author

We can and should keep the code generator itself on 21. We only need to target producing 17 bytecode for the client shapes.

Not sure I follow. If we intend to support customers on JDK17 then it seems like the the code generator plugin should be built on that version. If the code generator plugin targets JDK21 then customers will need to have a JDK21 toolchain configured just to execute the build tooling which seems like it could be a confusing/suboptimal customer experience. Is there some reason I am missing to keep the generator on 21?

@rhernandez35
Copy link
Contributor

We can and should keep the code generator itself on 21. We only need to target producing 17 bytecode for the client shapes.

Not sure I follow. If we intend to support customers on JDK17 then it seems like the the code generator plugin should be built on that version. If the code generator plugin targets JDK21 then customers will need to have a JDK21 toolchain configured just to execute the build tooling which seems like it could be a confusing/suboptimal customer experience. Is there some reason I am missing to keep the generator on 21?

We control the JVM used to run the CLI, right? I thought it was bundled into smithy-cli's runtime image. Or does the smithy gradle plugin use the gradle daemon's JVM when run in gradle? If we're in total control of the JVM used to run the smithy tooling, then we can do whatever we want in the plugins.

@hpmellema
Copy link
Contributor Author

We control the JVM used to run the CLI, right? I thought it was bundled into smithy-cli's runtime image. Or does the smithy gradle plugin use the gradle daemon's JVM when run in gradle? If we're in total control of the JVM used to run the smithy tooling, then we can do whatever we want in the plugins.

When we are running the build with Gradle the Smithy build tasks will use the Gradle daemon's JVM for executing the build plugins. This is because the Smithy Gradle plugins use the CLI as a JAR dependency rather than using the bundled CLI binary.

@hpmellema hpmellema merged commit 16822a9 into smithy-lang:main Apr 26, 2024
3 checks passed
@hpmellema hpmellema deleted the downgrade-codegen branch April 26, 2024 17:54
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.

3 participants