-
Notifications
You must be signed in to change notification settings - Fork 2
misc: upgrade to Gradle 9.0.0 #114
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
```
An exception occurred applying plugin request [id: 'org.jetbrains.kotlin.jvm']
> Failed to apply plugin class 'org.gradle.api.plugins.JavaPlugin'.
> Could not create domain object 'java' (JvmSoftwareComponentInternal) in SoftwareComponent container
> Cannot add a configuration with name 'runtimeClasspath' as a configuration with that name already exists.
```
| implementation(libs.smithy.model) | ||
| implementation(libs.smithy.gradle.base.plugin) | ||
| testImplementation(libs.junit.jupiter) | ||
| testImplementation(kotlin("test")) |
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 should use Kotlin's test instead of Jupiter regardless but was this because of the Gradle upgrade?
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.
Yes it was
| listOf( | ||
| "smithyCli", | ||
| "smithyBuild", | ||
| "runtimeClasspath", |
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 this removal due to the Gradle upgrade or just cleanup?
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.
Yes it was, the commit message has the exception I was seeing when trying to build aws-sdk-kotlin: 9115bb1
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.
Got it, but it doesn't break anything when we remove it? I'm guessing that it was there for a reason
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.
Nothing broke locally
| @@ -1 +1,4 @@ | |||
| kotlin.code.style=official | |||
|
|
|||
| # TODO Remove once configuration cache is enabled by default | |||
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.
When would that be?
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.
No clue
| kotlin.code.style=official | ||
|
|
||
| # TODO Remove once configuration cache is enabled by default | ||
| org.gradle.configuration-cache=true |
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'm looking at the documentation for 8.14.2 and it's not enabled by default. Why do we want to start using it now?
This feature is not enabled by default
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.
Because it's a new feature that improves build speed and has no downside
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.