Skip to content

Conversation

@InsertCreativityHere
Copy link
Member

@InsertCreativityHere InsertCreativityHere commented Oct 29, 2025

In the last week I've made some changes to the ice repo's gradle setup.
This PR copies the meaningful changes to the ice-demo repo to keep them in sync.

See zeroc-ice/ice#4616 and zeroc-ice/ice#4597

Push-Location $_.DirectoryName
Write-Host "Building in $($_.DirectoryName)..."
& ./gradlew rewriteDryRun
& ./gradlew rewriteDryRun --no-parallel --no-configuration-cache
Copy link
Member Author

Choose a reason for hiding this comment

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

rewriteDryRun can't be run in parallel or with the configuration cache.

"**/*.kt",
)
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Quality of life - so rewrite prints it's suggestions to the build output so they can be picked up.

<property name="allowNonPrintableEscapes" value="true"/>
</module>

<module name="UnusedImports"/>
Copy link
Member Author

Choose a reason for hiding this comment

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

No reason not to check for this.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the Java linting configuration and workflow to improve code quality checks. It adds unused import detection to Checkstyle, enhances the OpenRewrite dry run behavior to better communicate required changes, and improves the CI workflow reliability.

  • Added UnusedImports module to Checkstyle configuration for detecting unused imports
  • Enhanced rewriteDryRun task to display required changes and fail builds in CI when changes are needed
  • Updated CI workflow to run rewriteDryRun with --no-parallel and --no-configuration-cache flags

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
java/config/checkstyle/checkstyle.xml Added UnusedImports module to detect unused Java imports
java/build-logic/src/main/kotlin/zeroc-linting.gradle.kts Enhanced rewriteDryRun task with report cleanup, change display, and CI failure logic
.github/workflows/java.yml Added --no-parallel and --no-configuration-cache flags to rewriteDryRun command

Comment on lines +50 to +59
doFirst {
// Delete the old report file if present
val reportFile = file(reportPath)
if (reportFile.exists()) {
reportFile.delete()
}
}

doLast {
if (file(reportPath).exists()) {
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The property reportPath is accessed but not defined in this file. This will cause a compilation error unless reportPath is a property of RewriteDryRunTask. Verify that reportPath exists on the task or define it explicitly.

Suggested change
doFirst {
// Delete the old report file if present
val reportFile = file(reportPath)
if (reportFile.exists()) {
reportFile.delete()
}
}
doLast {
if (file(reportPath).exists()) {
// Define the path to the rewrite dry run report file
val reportPath = project.layout.buildDirectory.file("rewrite/rewrite-dry-run.txt").get().asFile.toPath()
doFirst {
// Delete the old report file if present
val reportFile = reportPath.toFile()
if (reportFile.exists()) {
reportFile.delete()
}
}
doLast {
if (reportPath.toFile().exists()) {

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

It's a property defined by the rewrite Task itself.

It's getter is defined here: https://github.com/openrewrite/rewrite-gradle-plugin/blob/363bbc7a63a2df4d8f78f33d6812e416ef9e77cf/plugin/src/main/java/org/openrewrite/gradle/RewriteDryRunTask.java#L32
To access them through gradle you omit the get prefix though.

@InsertCreativityHere InsertCreativityHere merged commit 7887e82 into zeroc-ice:main Oct 29, 2025
21 checks passed
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