Skip to content

Conversation

Pankraz76
Copy link

@Pankraz76 Pankraz76 commented Jul 18, 2025

apply:

fix:

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

related to:

Copy link
Contributor

❌ Gradle check result for bc07306: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@Pankraz76
Copy link
Author

is this any good? @owaiskazi19

on a large scale static code analysis really pays off. Paying single price instead of constant cost of carry.

Thanks to the new gen of tools like spot they convert the annoyance and risk to easy fixed delivered upfront.

@Pankraz76 Pankraz76 force-pushed the pr-RemoveUnusedPrivateMethods branch from bc07306 to 8872b73 Compare July 18, 2025 14:15
Copy link
Contributor

❌ Gradle check result for 8872b73: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@Pankraz76 Pankraz76 force-pushed the pr-RemoveUnusedPrivateMethods branch from 8872b73 to 5753137 Compare July 18, 2025 14:23
Copy link
Contributor

❌ Gradle check result for 5753137: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@owaiskazi19
Copy link
Member

@andrross @cwperks can you take a look at this change as well?

@Pankraz76
Copy link
Author

Thanks a lot. I have given up, as the plugin changed.

From working on a 8gb machine in 40 mins to running "endless" without maxing out CPU.

@timtebeek do you have an idea what´s changed recently, causing this performance difference? As @hazendaz has pointed out correctly, every tool, including spotbugs, has some bugs.

I will try to cover this as well like recent discovery suggested by @hazendaz.
Again thanks for the contribution making rewrite better by the day.

Is that because of a problem with the rewrite gradle plugin, or something related to our setup?

not sure about that, most projects have some parsing error´s upfront, until its going into running recipe mode. Thats something advanced @timtebeek need to check out further.

Appreciate it.

@Pankraz76 Pankraz76 changed the title Apply RemoveUnusedPrivateMethods Introduce Rewrite fixing S1144: Unused "private" methods should be removed Aug 29, 2025
@Pankraz76 Pankraz76 changed the title Introduce Rewrite fixing S1144: Unused "private" methods should be removed Introduce Rewrite & PMD fixing S1144: Unused "private" methods should be removed Aug 30, 2025
@Pankraz76 Pankraz76 force-pushed the pr-RemoveUnusedPrivateMethods branch 2 times, most recently from ddaec8b to 6a7ddb7 Compare August 30, 2025 12:04
@@ -76,9 +76,9 @@ Fork [opensearch-project/OpenSearch](https://github.com/opensearch-project/OpenS

#### JDK

OpenSearch recommends building with the [Temurin/Adoptium](https://adoptium.net/temurin/releases/) distribution. JDK 11 is the minimum supported, and JDK-24 is the newest supported. You must have a supported JDK installed with the environment variable `JAVA_HOME` referencing the path to Java home for your JDK installation, e.g. `JAVA_HOME=/usr/lib/jvm/jdk-21`.
Copy link
Author

Choose a reason for hiding this comment

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

can not undo im sorry. will do if requested in final commit version by using different editor.

Copy link
Contributor

❌ Gradle check result for 6a7ddb7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

rewrite("org.openrewrite.recipe:rewrite-static-analysis:2.15.0")
}

tasks.check.dependsOn(rewriteDryRun)
Copy link
Author

Choose a reason for hiding this comment

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

i think its not possible to run this no default on dev machine.

Rewrite run should be the roundup solution to fix all the issues breaking the build/quality gate, just like already achieved with spotless.

PMD is fast enough to run locally on the other hand giving quick indication for broad errors, as some are in common. Other details recipes can not be covered of course, given the need to rewrite to run (in cloud).

@Pankraz76 Pankraz76 force-pushed the pr-RemoveUnusedPrivateMethods branch from 6a7ddb7 to f2b52bb Compare August 30, 2025 13:00
Copy link
Contributor

❌ Gradle check result for f2b52bb: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@Pankraz76 Pankraz76 force-pushed the pr-RemoveUnusedPrivateMethods branch from f2b52bb to 4100ee8 Compare August 30, 2025 13:26
Copy link
Contributor

❌ Gradle check result for 4100ee8: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@Pankraz76
Copy link
Author

Thanks @Pankraz76. I checked out this PR to try it out, but ran into a few issues. I ran:

./gradlew rewriteRun -Dorg.gradle.jvmargs=-Xmx16G

I had to use a 16G heap otherwise I ran out of memory. It found unused private methods in 36 files. However, a subsequent compilation failed because it removed the method StarTreeMapper::Builder::findMapperBuilderByName even though that method is still used. I reverted that change and then compilation failed because the method SearchAfterIT::convertSortValues had been removed. Is this expected?

Also, I get an enormous output of warnings like the following when running the tool:

Resolution of the configuration :modules:ingest-user-agent:jacocoAnt was attempted from a context different than the project context. Have a look at the documentation to understand why this is a problem and how it can be resolved. This behavior has been deprecated. This will fail with an error in Gradle 9.0. For more information, please refer to https://docs.gradle.org/8.14.3/userguide/viewing_debugging_dependencies.html#sub:resolving-unsafe-configuration-resolution-errors in the Gradle documentation.

Is that because of a problem with the rewrite gradle plugin, or something related to our setup?

Updated the config accordingly. Please consider retest my simply executing ./gradlew rewriteDryRun -Dorg.gradle.jvmargs=-Xmx8G with the now clickable action in the developer guide:

image

This should work and sort out the smells targeted with PMD and automated with rewrite. Its just as with checkstyle and spotless there is a natural evolution in this. Could argue to keep both as each having its benefits and weaknesses, meaning for the same thing both finding stuff the other one dont making it truly a win. As rewrite takes so long PMD is a good candidate to run on dev while the big bang and mighty rewrite runs on CI.

As @hazendaz has correctly pointed out that each tool having its benefits, making each a a win for every project, alone and of course having the biggest impact in combination.

@Pankraz76 Pankraz76 changed the title Introduce Rewrite & PMD fixing S1144: Unused "private" methods should be removed Introduce Rewrite & PMD covering S1144: Unused "private" methods should be removed Aug 30, 2025
@Pankraz76 Pankraz76 force-pushed the pr-RemoveUnusedPrivateMethods branch from 4100ee8 to b41c35a Compare August 30, 2025 13:35
Copy link
Contributor

❌ Gradle check result for b41c35a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@hazendaz
Copy link

I am not involved with this user @Pankraz76. I kindly ask this user stop using my name.

@Pankraz76
Copy link
Author

I am not involved with this user @Pankraz76. I kindly ask this user stop using my name.

please excuse me.

Just wanted to pay the credits accordingly, as supported rewrite and PMD as well.

Thanks.

@Pankraz76 Pankraz76 force-pushed the pr-RemoveUnusedPrivateMethods branch from b41c35a to 297f424 Compare August 31, 2025 18:38
Copy link
Contributor

❌ Gradle check result for 297f424: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@Pankraz76
Copy link
Author

Kindly request some feedback on how to proceed here, thanks. @andrross

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.

4 participants