Issue #18470: Align `spotbugs` and `pmd` config with `rewrite` #18454 #18479 by Pankraz76 · Pull Request #18440 · checkstyle/checkstyle

Pankraz76 · 2025-12-27T12:00:01Z

Issue #18470: Align `spotbugs` and `pmd` config with `rewrite` #18454 #18479

romani · 2025-12-27T19:39:50Z

I don't understand problem

Pankraz76 · 2025-12-27T19:45:12Z

yes, its strange that no other PR seems to build. But different PR seems fine so lets observe.

Pankraz76 · 2025-12-28T10:40:03Z

-  cd .ci-temp/spotbugs-and-pmd
+  ./mvnw -e --no-transfer-progress clean test-compile pmd:check
+  cd .ci-temp/pmd
  grep "Processing_Errors" "$CHECKSTYLE_DIR/target/site/pmd.html" | cat > errors.log


do we actually need this extra extra ?

Pankraz76 · 2025-12-28T10:40:50Z

  ;;

-pmd)
-  ./mvnw -e --no-transfer-progress clean test-compile pmd:check


why not just use simple check like in spotbugs and spotless.

We already on happy path being complain. All we do is baby steps not on local dev env.

romani

I don't understand reason of update.
If you will not stop doing unrelated updates, I will put you in ignore list, please keep your productivity on leash.

Pankraz76 · 2025-12-30T06:47:24Z

I don't understand reason of update. If you will not stop doing unrelated updates, I will put you in ignore list, please keep your productivity on leash.

we having lots of custom overhead code for no reason. Look into changes. We doing some crazy pmd stuff you dont even notice or want to explain.

The code just makes not sense when looking into detail. I know some dont understand the detail, but they matter most!

The is just working but on a very strange and costly way, there is actually no need to have this kind of silliness around.

I will put you in ignore list

no worry, I will leave the project anytime soon!

Pankraz76 · 2025-12-30T06:53:19Z

this a low hanging fruit ready to be merged. Having made extra comments everywhere. If you dont want to consider just tell me but dont make excuses you dont know whats going no. Im trying to help here and make the project leaner avoiding its questions without any answers given...

Pankraz76 · 2025-12-30T09:32:15Z

    -Dpmd.skip=true -Dspotbugs.skip=true -Djacoco.skip=true
  ;;

-spotbugs-and-pmd)


why having this overhead? can we not use simple KISS prinicple?

CI job start will cost more than execution of job, so grouping is happening

yes thats fine. Still its the question to spend 30 sek compiling to not have a dedicated feedback. If the build takes several mins. i would see that point more critical.

why having this overhead? can we not use simple KISS prinicple?

also im not asking directly to split, but why having the file handling for pmd? We just want to call the check goal, thats it.

Pankraz76 · 2025-12-30T10:15:41Z

      # https://github.com/spotbugs/spotbugs-maven-plugin/issues/806 explains
      # why we need execution of spotbugs without any other maven plugins which can change binaries
      - validate-with-maven-script:
-          name: "spotbugs-and-pmd"


this the main benefit and change of PR. Single concern principle.

Pankraz76 · 2025-12-31T09:44:05Z

  echo "Checkout target sources ..."
  checkout_from "https://github.com/pmd/build-tools.git"
  cd .ci-temp/build-tools/
-  mvn -e --no-transfer-progress install


maybe should use same mvn instance all over

Pankraz76 · 2025-12-31T09:45:14Z

+  echo "Spotbugs"
+  ./mvnw -e --no-transfer-progress spotbugs:check
+  echo "PMD"
+  ./mvnw -e --no-transfer-progress pmd:check


does maven cash the compile phase somehow? it would be nice to have dedicated calls , if not merge again.

Pankraz76 · 2025-12-31T09:45:32Z

-          image-name: "cimg/openjdk:17.0.7"
-          command: "./.ci/no-exception-test.sh no-exception-samples-gradle"
-      - validate-with-maven-script:
-          name: "no-exception-samples-maven"


having moved to exception block

Pankraz76 · 2025-12-31T10:12:20Z

+          command: ./.ci/validation.sh git-check-pull-number
  cli-validation:
    jobs:
-      - yamllint


you asked how it would be simpler imho if possible this would help: With the colon : it has same style and does not look lost anymore.

Pankraz76 · 2026-01-01T10:03:18Z

  exit $fail
  ;;

-checkstyle-and-sevntu)


and = coupling which seems the forbidden fruit all around in arch software design. Why should we not follow this fundamental principe here as well? This is actually the root cause of this issue so we should try to keep it real...

Pankraz76 · 2026-01-01T10:04:30Z

  export MAVEN_OPTS='-Xmx2g'
-  ./mvnw -e --no-transfer-progress clean verify -DskipTests -DskipITs \
-    -Dpmd.skip=true -Dspotbugs.skip=true -Djacoco.skip=true
+  ./mvnw -e --no-transfer-progress clean checkstyle:check


idk but this smells, as we fail to apply convention principle:

Caused by: org.apache.maven.plugin.MojoFailureException: You have 13302 Checkstyle violations.

Pankraz76 · 2026-01-01T10:57:36Z

-          command: "./.ci/validation.sh no-error-test-sbe"
+          name: sevntu
+          image-name: *cs_img
+          command: ./.ci/validation.sh sevntu


this one not working as expected, also its called somwhere else. Idk if we have to keep it here. It does not work locally.

Pankraz76 · 2026-01-01T11:58:32Z

      -Xep:ExplicitEnumOrdering:ERROR
      -Xep:FormatStringConcatenation:ERROR
-      -Xep:IdentityConversion:ERROR
+      <!-- -Xep:IdentityConversion:ERROR -->


issue:

Error Prone Plugin Not Applied and Build Passes Despite Errors #18470 #18479

Pankraz76 · 2026-01-01T11:58:53Z

                  <goal>compile</goal>
                </goals>
                <configuration>
-                  <failOnError>false</failOnError>


why not keep it simple stupid?

also convention principle.

Pankraz76 · 2026-01-01T11:59:49Z

+#
+#      - name: Execute Error-Prone on test-compile phase
+#        if: always()
+#        run: groovy ./.ci/error-prone-check.groovy test-compile


avoiding all this extra extra ?

Once we good to go and passing CI, everything is done locally. Maybe not need this just fail on error.

actually would have expected @rickie to be around. Whats you call on this, how should we integrate this proper.

AFAIK the CI does correctly flag it when Error Prone finds something, so not sure if there is a problem here.

Pankraz76 · 2026-01-01T12:20:24Z

item

Run docker://rhysd/actionlint:latest
/usr/bin/docker run --name rhysdactionlintlatest_10391d --label 4620a9 --workdir /github/workspace --rm -e "HOME" -e "GITHUB_JOB" -e "GITHUB_REF" -e "GITHUB_SHA" -e "GITHUB_REPOSITORY" -e "GITHUB_REPOSITORY_OWNER" -e "GITHUB_REPOSITORY_OWNER_ID" -e "GITHUB_RUN_ID" -e "GITHUB_RUN_NUMBER" -e "GITHUB_RETENTION_DAYS" -e "GITHUB_RUN_ATTEMPT" -e "GITHUB_ACTOR_ID" -e "GITHUB_ACTOR" -e "GITHUB_WORKFLOW" -e "GITHUB_HEAD_REF" -e "GITHUB_BASE_REF" -e "GITHUB_EVENT_NAME" -e "GITHUB_SERVER_URL" -e "GITHUB_API_URL" -e "GITHUB_GRAPHQL_URL" -e "GITHUB_REF_NAME" -e "GITHUB_REF_PROTECTED" -e "GITHUB_REF_TYPE" -e "GITHUB_WORKFLOW_REF" -e "GITHUB_WORKFLOW_SHA" -e "GITHUB_REPOSITORY_ID" -e "GITHUB_TRIGGERING_ACTOR" -e "GITHUB_WORKSPACE" -e "GITHUB_ACTION" -e "GITHUB_EVENT_PATH" -e "GITHUB_ACTION_REPOSITORY" -e "GITHUB_ACTION_REF" -e "GITHUB_PATH" -e "GITHUB_ENV" -e "GITHUB_STEP_SUMMARY" -e "GITHUB_STATE" -e "GITHUB_OUTPUT" -e "RUNNER_OS" -e "RUNNER_ARCH" -e "RUNNER_NAME" -e "RUNNER_ENVIRONMENT" -e "RUNNER_TOOL_CACHE" -e "RUNNER_TEMP" -e "RUNNER_WORKSPACE" -e "ACTIONS_RUNTIME_URL" -e "ACTIONS_RUNTIME_TOKEN" -e "ACTIONS_CACHE_URL" -e "ACTIONS_RESULTS_URL" -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp":"/github/runner_temp" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/runner/work/checkstyle/checkstyle":"/github/workspace" rhysd/actionlint:latest
.github/workflows/diff-report.yml:59:13: constant expression "true" in condition. remove the if: section [if-cond]
   |
59 |         if: 'true'
   |             ^~~~~~

Pankraz76 · 2026-01-01T12:32:55Z

     * @return a version string based on the package implementation version
     */
    private static String replaceVersionString(String report) {
-        final String version = SarifLogger.class.getPackage().getImplementationVersion();


The “trillion” example shows why this kind of coupling nonsense feels pretty clunky.

At that point, the code makes little sense; it’s mostly written that way because of the coupling. Of course, it could be done in a functional, inline style, but that’s unlikely, since it would blow up the space and hurt readability.

It also shows that this super noisy, repetitive typing obviously wasn’t enough to make it clear that String.valueOf(String) is not a great idea.

So after just one line, we’ve already forgotten all the context. That’s why doing things inline in small steps, with high cohesion, always seems like a smart idea, IMHO.

…te` checkstyle#18454 checkstyle#18479

romani · 2026-01-02T05:22:18Z

I lost a purpose of this PR

Pankraz76 force-pushed the fix-ci branch from e6f1dbf to 087a7bb Compare December 27, 2025 12:01

Pankraz76 commented Dec 27, 2025

View reviewed changes

Comment thread .ci/validation.sh Outdated

Pankraz76 marked this pull request as ready for review December 27, 2025 12:05

Pankraz76 marked this pull request as draft December 27, 2025 12:57

Pankraz76 force-pushed the fix-ci branch from deb2f59 to 45e4053 Compare December 27, 2025 13:03

Pankraz76 marked this pull request as ready for review December 27, 2025 13:16

Pankraz76 mentioned this pull request Dec 27, 2025

Issue #17487: Add ECMAScript6BestPractices #18158

Closed

Pankraz76 changed the title ~~Issue #16543: Fix CI, adjust spotless config~~ Issue #16543: Dedicate spotbugs and pmd CI config Dec 28, 2025

Pankraz76 commented Dec 28, 2025

View reviewed changes

Pankraz76 force-pushed the fix-ci branch from 45e4053 to c0c4990 Compare December 28, 2025 10:44

Pankraz76 commented Dec 28, 2025

View reviewed changes

Comment thread .circleci/config.yml

Comment thread .circleci/config.yml

Comment thread .circleci/config.yml

Comment thread .circleci/config.yml Outdated

Comment thread .ci/validation.sh Outdated

Pankraz76 force-pushed the fix-ci branch from c0c4990 to 089317d Compare December 29, 2025 09:30

romani requested changes Dec 29, 2025

View reviewed changes

Pankraz76 requested a review from romani December 30, 2025 06:52

Pankraz76 changed the title ~~Issue #16543: Dedicate spotbugs and pmd CI config~~ Issue #16543: Align spotbugs and pmd config with rewrite Dec 30, 2025

Pankraz76 force-pushed the fix-ci branch from 089317d to b0de703 Compare December 30, 2025 09:31

Pankraz76 commented Dec 30, 2025

View reviewed changes

Pankraz76 force-pushed the fix-ci branch 3 times, most recently from c912af1 to 01f56e4 Compare December 30, 2025 10:14

Pankraz76 commented Dec 30, 2025

View reviewed changes

Pankraz76 force-pushed the fix-ci branch 4 times, most recently from ae58156 to ad69697 Compare December 31, 2025 10:09

Pankraz76 commented Dec 31, 2025

View reviewed changes

Pankraz76 pushed a commit to Pankraz76/checkstyle that referenced this pull request Jan 1, 2026

Issue checkstyle#18470: Cleanup CI config checkstyle#18440

8e84831

Pankraz76 pushed a commit to Pankraz76/checkstyle that referenced this pull request Jan 1, 2026

Issue checkstyle#18470: Cleanup CI config checkstyle#18440

33ccd80

Pankraz76 pushed a commit to Pankraz76/checkstyle that referenced this pull request Jan 1, 2026

Issue checkstyle#18470: Cleanup CI config checkstyle#18440

82d5a38

Pankraz76 pushed a commit to Pankraz76/checkstyle that referenced this pull request Jan 1, 2026

Issue checkstyle#18470: Cleanup CI config checkstyle#18440

ff99056

Pankraz76 pushed a commit to Pankraz76/checkstyle that referenced this pull request Jan 1, 2026

Issue checkstyle#18470: Cleanup CI config checkstyle#18440

5121069

Pankraz76 force-pushed the fix-ci branch from ff99056 to 5121069 Compare January 1, 2026 10:00

Pankraz76 commented Jan 1, 2026

View reviewed changes

Pankraz76 mentioned this pull request Jan 1, 2026

Issue #17487: Add EmptyNewlineAtEndOfFile #18477

Closed

Pankraz76 pushed a commit to Pankraz76/checkstyle that referenced this pull request Jan 1, 2026

Issue checkstyle#18470: Cleanup CI config checkstyle#18440

8dae99c

Pankraz76 force-pushed the fix-ci branch 2 times, most recently from 23d37e0 to 459966b Compare January 1, 2026 10:56

Pankraz76 commented Jan 1, 2026

View reviewed changes

Pankraz76 force-pushed the fix-ci branch from 459966b to f904259 Compare January 1, 2026 11:02

Pankraz76 mentioned this pull request Jan 1, 2026

Error Prone Plugin Not Applied and Build Passes Despite Errors #18470 #18479

Closed

Pankraz76 force-pushed the fix-ci branch from 3a5b33a to eaa8f07 Compare January 1, 2026 11:42

Pankraz76 changed the title ~~Issue #18470: Align spotbugs and pmd config with rewrite #18454~~ Issue #18470: Align spotbugs and pmd config with rewrite #18454 #18479 Jan 1, 2026

Pankraz76 force-pushed the fix-ci branch 2 times, most recently from 8bc3b31 to 5913649 Compare January 1, 2026 11:49

Pankraz76 commented Jan 1, 2026

View reviewed changes

Pankraz76 pushed a commit to Pankraz76/checkstyle that referenced this pull request Jan 1, 2026

Issue checkstyle#18470: Cleanup CI config checkstyle#18440

7ade545

Pankraz76 force-pushed the fix-ci branch 2 times, most recently from afc280e to 0acf198 Compare January 1, 2026 12:27

Pankraz76 commented Jan 1, 2026

View reviewed changes

Issue checkstyle#18470: Align spotbugs and pmd config with `rewri…

5de4459

…te` checkstyle#18454 checkstyle#18479

Pankraz76 force-pushed the fix-ci branch from 0acf198 to 5de4459 Compare January 1, 2026 12:43

romani closed this Jan 2, 2026

Uh oh!

Conversation

Pankraz76 commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!