Skip to content

TEZ-4691: Upgrade maven-checkstyle-version to 3.6.0 - #463

Merged
abstractdog merged 2 commits into
apache:masterfrom
Aggarwal-Raghav:TEZ-4691
Mar 11, 2026
Merged

TEZ-4691: Upgrade maven-checkstyle-version to 3.6.0#463
abstractdog merged 2 commits into
apache:masterfrom
Aggarwal-Raghav:TEZ-4691

Conversation

@Aggarwal-Raghav

Copy link
Copy Markdown
Contributor

Check TEZ-4691 for details

Comment threadpom.xml
<build-helper-maven-plugin.version>1.8</build-helper-maven-plugin.version>
<buildnumber-maven-plugin.version>1.1</buildnumber-maven-plugin.version>
<checkstyle.version>8.35</checkstyle.version>
<checkstyle.version>13.3.0</checkstyle.version>

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I didn't removed puppycrawler explicit dependency as it is present in offical doc as well and based on google maven-checkstyle-version have 9.3 as default puppycrawler version. For now I have moved it to latest 13.3.0.
https://maven.apache.org/plugins/maven-checkstyle-plugin/examples/upgrading-checkstyle.html

@abstractdog

Copy link
Copy Markdown
Contributor

LGTM, pending tests

@abstractdog
abstractdog self-requested a review March 9, 2026 11:06
@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

Will check the first CI run to confirm if checkstyle fails because of plexus-velocity or not

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

@abstractdog , tez CI is asking for login (seeing it for first time). How to login/bypass it?
Screenshot 2026-03-09 at 7 51 44 PM

@abstractdog

Copy link
Copy Markdown
Contributor

@abstractdog , tez CI is asking for login (seeing it for first time). How to login/bypass it? Screenshot 2026-03-09 at 7 51 44 PM

not sure, have to check what changed on ci-hadoop

I'm monitoring the corresponding job in the meantime, but it's not yet started, because jenkins is about to shut down
Screenshot 2026-03-09 at 15 44 20

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec17m 59sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+0 🆗xmllint0m 0sxmllint was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚mvninstall11m 40smaster passed
+1 💚compile3m 16smaster passed
+1 💚javadoc1m 43smaster passed
_ Patch Compile Tests _
+1 💚mvninstall4m 54sthe patch passed
+1 💚codespell2m 14sNo new issues.
+1 💚compile2m 33sthe patch passed
+1 💚javac2m 33sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚javadoc1m 20sthe patch passed
_ Other Tests _
+1 💚unit73m 31sroot in the patch passed.
+1 💚asflicense0m 35sThe patch does not generate ASF License warnings.
121m 21s
SubsystemReport/Notes
DockerClientAPI=1.54 ServerAPI=1.54 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/1/artifact/out/Dockerfile
GITHUB PR#463
Optional Testsdupname asflicense javac javadoc unit codespell detsecrets xmllint compile
unameLinux 6a5d39947b97 5.15.0-164-generic #174-Ubuntu SMP Fri Nov 14 20:25:16 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personality/home/jenkins/jenkins-agent/workspace/tez-multibranch_PR-463/src/.yetus/personality.sh
git revisionmaster / 7dd38f9
Default JavaUbuntu-21.0.10+7-Ubuntu-124.04
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/1/testReport/
Max. process+thread count1335 (vs. ulimit of 5500)
modulesC: . U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/1/console
versionsgit=2.43.0 maven=3.8.7 codespell=2.4.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@abstractdog

abstractdog commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

@Aggarwal-Raghav : a bit concerned that checkstyle hasn't been triggered by this commit: can you please add an artifical style issue (e.g. bad indentation to a random class) to see if it fails and correctly reports 1 additional issue (I see how uncomfortable to bother with commits like this, this is what it is at the moment)

if it works, you can remove the extra commit and we can merge ASAP

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

@Aggarwal-Raghav : a bit concerned that checkstyle hasn't been triggered by this commit: can you please add an artifical style issue (e.g. bad indentation to a random class) to see if it fails and correctly reports 1 additional issue (I see how uncomfortable to bother with commits like this, this is what it is at the moment)

if it works, you can remove the extra commit and we can merge ASAP

no worries at all. one doubt, if i make any java changes then it will trigger right? Maybe I can fix 1 checkstyle in a java class 😅

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec7m 28sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+0 🆗xmllint0m 0sxmllint was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 1 new or modified test files.
_ master Compile Tests _
+0 🆗mvndep2m 25sMaven dependency ordering for branch
+1 💚mvninstall10m 9smaster passed
+1 💚compile3m 7smaster passed
+1 💚checkstyle1m 28smaster passed
+1 💚javadoc1m 58smaster passed
+0 🆗spotbugs2m 34stez-dag in master has 749 extant spotbugs warnings.
+0 🆗spotbugs7m 49sroot in master has 1935 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 13sMaven dependency ordering for patch
+1 💚mvninstall5m 30sthe patch passed
+1 💚codespell1m 48sNo new issues.
+1 💚compile3m 14sthe patch passed
+1 💚javac3m 14sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
-0 ⚠️checkstyle0m 36s/buildtool-patch-checkstyle-tez-dag.txtThe patch fails to run checkstyle in tez-dag
-0 ⚠️checkstyle0m 19s/buildtool-patch-checkstyle-root.txtThe patch fails to run checkstyle in root
+1 💚javadoc1m 50sthe patch passed
+1 💚spotbugs9m 59sthe patch passed
_ Other Tests _
-1 ❌unit6m 34s/patch-unit-tez-dag.txttez-dag in the patch passed.
-1 ❌unit75m 20s/patch-unit-root.txtroot in the patch passed.
+1 💚asflicense0m 56sThe patch does not generate ASF License warnings.
146m 8s
ReasonTests
Failed junit teststez.dag.history.recovery.TestRecoveryService
tez.dag.history.recovery.TestRecoveryService
SubsystemReport/Notes
DockerClientAPI=1.54 ServerAPI=1.54 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/2/artifact/out/Dockerfile
GITHUB PR#463
Optional Testsdupname asflicense javac javadoc unit codespell detsecrets xmllint compile spotbugs checkstyle
unameLinux fc48d4c5e33b 5.15.0-164-generic #174-Ubuntu SMP Fri Nov 14 20:25:16 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personality/home/jenkins/jenkins-home/workspace/tez-multibranch_PR-463/src/.yetus/personality.sh
git revisionmaster / b7dddb9
Default JavaUbuntu-21.0.10+7-Ubuntu-124.04
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/2/testReport/
Max. process+thread count1422 (vs. ulimit of 5500)
modulesC: tez-dag . U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/2/console
versionsgit=2.43.0 maven=3.8.7 spotbugs=4.9.3 codespell=2.4.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@abstractdog

Copy link
Copy Markdown
Contributor

https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/2/artifact/out/buildtool-patch-checkstyle-tez-dag.txt
oh no, this is something similar to what we saw in case of spotbugs in TEZ-4669
#457

do you have the bandwidth to investigate it @Aggarwal-Raghav ?

@Aggarwal-Raghav

Aggarwal-Raghav commented Mar 10, 2026

Copy link
Copy Markdown
ContributorAuthor

https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/2/artifact/out/buildtool-patch-checkstyle-tez-dag.txt
oh no, this is something similar to what we saw in case of spotbugs in TEZ-4669
#457

do you have the bandwidth to investigate it @Aggarwal-Raghav ?

Yes, I know this and observed in #462 as well. Just wanted to confirm without apache 37 parent pom it is happening. Will update PR shortly

#463 (comment)

@tez-yetus

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 15sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+0 🆗xmllint0m 0sxmllint was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 2 new or modified test files.
_ master Compile Tests _
+0 🆗mvndep2m 31sMaven dependency ordering for branch
+1 💚mvninstall10m 18smaster passed
+1 💚compile3m 20smaster passed
+1 💚checkstyle1m 25smaster passed
+1 💚javadoc1m 56smaster passed
+0 🆗spotbugs2m 24stez-dag in master has 749 extant spotbugs warnings.
+0 🆗spotbugs7m 19sroot in master has 1935 extant spotbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 13sMaven dependency ordering for patch
+1 💚mvninstall5m 8sthe patch passed
+1 💚codespell1m 48sNo new issues.
+1 💚compile3m 4sthe patch passed
+1 💚javac3m 4sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
-0 ⚠️checkstyle0m 51s/results-checkstyle-tez-dag.txttez-dag: The patch generated 2 new + 66 unchanged - 4 fixed = 68 total (was 70)
-0 ⚠️checkstyle0m 58s/results-checkstyle-root.txtroot: The patch generated 2 new + 66 unchanged - 4 fixed = 68 total (was 70)
+1 💚javadoc1m 51sthe patch passed
+1 💚spotbugs9m 16sthe patch passed
_ Other Tests _
+1 💚unit6m 2stez-dag in the patch passed.
+1 💚unit71m 42sroot in the patch passed.
+1 💚asflicense1m 3sThe patch does not generate ASF License warnings.
134m 36s
SubsystemReport/Notes
DockerClientAPI=1.54 ServerAPI=1.54 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/3/artifact/out/Dockerfile
GITHUB PR#463
Optional Testsdupname asflicense javac javadoc unit codespell detsecrets xmllint compile spotbugs checkstyle
unameLinux 21f09c8442b4 5.15.0-164-generic #174-Ubuntu SMP Fri Nov 14 20:25:16 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personality/home/jenkins/jenkins-home/workspace/tez-multibranch_PR-463/src/.yetus/personality.sh
git revisionmaster / 4aa2b66
Default JavaUbuntu-21.0.10+7-Ubuntu-124.04
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/3/testReport/
Max. process+thread count1358 (vs. ulimit of 5500)
modulesC: tez-dag . U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-463/3/console
versionsgit=2.43.0 maven=3.8.7 spotbugs=4.9.3 codespell=2.4.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@abstractdog

Copy link
Copy Markdown
Contributor

changes look good to me, thanks a lot @Aggarwal-Raghav for fixing plexus-velocity bug! will merge this soon

@abstractdog
abstractdog merged commit ea890f8 into apache:masterMar 11, 2026
4 checks passed
@Aggarwal-Raghav
Aggarwal-Raghav deleted the TEZ-4691 branch May 1, 2026 05:52
Sign up for freeto 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.

3 participants

@Aggarwal-Raghav@abstractdog@tez-yetus