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

Deprecations fixes and minor improvements in gradle scripts #10036

Merged
merged 3 commits into from
Nov 24, 2023

Conversation

altro3
Copy link
Contributor

@altro3 altro3 commented Oct 27, 2023

No description provided.

@altro3 altro3 force-pushed the gradle-fixes branch 3 times, most recently from 75ba45b to a0a28d6 Compare October 27, 2023 08:31
@sdelamo sdelamo requested a review from melix November 10, 2023 10:27
@sdelamo sdelamo added this to the 4.2.0 milestone Nov 10, 2023
Copy link
Contributor

@melix melix left a comment

Choose a reason for hiding this comment

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

Looks almost good. I would revert the all changes which do not make sense and fix the layout things.

@@ -14,7 +14,7 @@ repositories {
mavenCentral()
}

configurations.all {
configurations.configureEach {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually not required. Configurations cannot be configured lazily. Actually using configureEach on the configuration container can lead to hard to debug errors, so I would simply revert these changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@melix I didi it, because Idea every time tell me, that is configureEach better than all:

изображение

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't always trust your IDE I guess ;)

@@ -5,7 +5,7 @@ plugins {
}

tasks.named("htmlSanityCheck") {
sourceDir = new File("${rootProject.buildDir}/docs/guide/")
sourceDir = file("${rootProject.layout.buildDirectory.asFile.get()}/docs/guide/")
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ This is a mix of file and layout.

Suggested change
sourceDir = file("${rootProject.layout.buildDirectory.asFile.get()}/docs/guide/")
sourceDir = rootProject.layout.buildDirectory.dir("docs/guide").get().asFile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -31,7 +31,7 @@ spotless {
}

def versionInfo = tasks.register("micronautVersionInfo", WriteProperties) {
outputFile = "${buildDir}/resources/version/micronaut-version.properties"
destinationFile = file("${layout.buildDirectory.asFile.get()}/resources/version/micronaut-version.properties")
Copy link
Contributor

Choose a reason for hiding this comment

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

destinationFile is a proper RegularFileProperty

Suggested change
destinationFile = file("${layout.buildDirectory.asFile.get()}/resources/version/micronaut-version.properties")
destinationFile = layout.buildDirectory.file("resources/version/micronaut-version.properties")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -48,7 +48,7 @@ graalvmNative {
enabled = true
}
binaries {
all {
configureEach {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, configureEach doesn't make sense since it's a model element, so it will register tasks in any case. This will never be lazy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

@sdelamo sdelamo modified the milestones: 4.2.0, 4.2.1 Nov 10, 2023
@sdelamo sdelamo added the relates-to: build label for issues related to the build file or CI label Nov 10, 2023
@altro3
Copy link
Contributor Author

altro3 commented Nov 11, 2023

I reverted all and fixed layout

@sdelamo sdelamo requested a review from melix November 14, 2023 06:17
@sdelamo sdelamo merged commit 5677ba8 into micronaut-projects:4.2.x Nov 24, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relates-to: build label for issues related to the build file or CI
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants