Skip to content

GH-39946: [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29 - #39694

Merged
lidavidm merged 6 commits into
apache:mainfrom
vibhatha:feat-puppycrawl-update-8.29
Feb 5, 2024
Merged

GH-39946: [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29#39694
lidavidm merged 6 commits into
apache:mainfrom
vibhatha:feat-puppycrawl-update-8.29

Conversation

@vibhatha

@vibhathavibhatha commented Jan 19, 2024

Copy link
Copy Markdown
Contributor

Rationale for this change

This PR was created in place of #39202 to integrate the puppycrawl.tools.checkstyle upgrade.

What changes are included in this PR?

Style changes in Java classes and core changes to the style format itself.
Some unsupported attributes have been removed. And some attributes have
been reorganized upon the provided guidelines in the documentation.

Are these changes tested?

N/A
Tested by existing checkstyle guideline.

Are there any user-facing changes?

No

@vibhatha

Copy link
Copy Markdown
ContributorAuthor

@github-actions crossbow submit java

Comment threadjava/dev/checkstyle/checkstyle.xml Outdated

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.

will remove these once CIs are verified.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this ready or not ready? Please keep PRs in draft until they are ready.

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.

This should have been on draft. Sorry about the mistake.

@github-actionsgithub-actionsBot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 19, 2024
@github-actions

Copy link
Copy Markdown

Revision: 671b530eceb7b629d979b09306faa9637fa0127e

Submitted crossbow builds: ursacomputing/crossbow @ actions-dd3d152807

TaskStatus
java-jarsGitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64GitHub Actions
verify-rc-source-java-macos-amd64GitHub Actions

@vibhatha

Copy link
Copy Markdown
ContributorAuthor

@github-actions crossbow submit java

@vibhatha

Copy link
Copy Markdown
ContributorAuthor

@github-actions

Copy link
Copy Markdown

Revision: 671b530eceb7b629d979b09306faa9637fa0127e

Submitted crossbow builds: ursacomputing/crossbow @ actions-7d4c086df1

TaskStatus
java-jarsGitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64GitHub Actions
verify-rc-source-java-macos-amd64GitHub Actions

@vibhathavibhatha changed the title MINOR: [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29 in /java/maven V2 [Java] Bump org.codehaus.mojo:exec-maven-plugin from 1.6.0 to 3.1.1Jan 22, 2024
@vibhathavibhatha changed the title [Java] Bump org.codehaus.mojo:exec-maven-plugin from 1.6.0 to 3.1.1MINOR: [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29 in /java/maven V2 Jan 22, 2024
@vibhathavibhatha changed the title MINOR: [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29 in /java/maven V2 [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29Jan 22, 2024
@vibhatha
vibhathaforce-pushed the feat-puppycrawl-update-8.29 branch from 671b530 to e07b445CompareJanuary 22, 2024 11:58
@vibhatha

Copy link
Copy Markdown
ContributorAuthor

@github-actions crossbow submit java

@github-actions

Copy link
Copy Markdown

Revision: e07b4453c3d2639f453c46b50c4664b132f7b977

Submitted crossbow builds: ursacomputing/crossbow @ actions-5f3cb7c61a

TaskStatus
java-jarsGitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64GitHub Actions
verify-rc-source-java-macos-amd64GitHub Actions

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jan 22, 2024
@vibhatha
vibhatha marked this pull request as draft January 23, 2024 06:48
@vibhatha
vibhathaforce-pushed the feat-puppycrawl-update-8.29 branch from e07b445 to 3a74b4dCompareJanuary 23, 2024 07:45
@vibhatha

Copy link
Copy Markdown
ContributorAuthor

@github-actions crossbow submit java

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jan 23, 2024
@github-actions

Copy link
Copy Markdown

Revision: 3a74b4dcd16894b2599e830e114590665bb43a94

Submitted crossbow builds: ursacomputing/crossbow @ actions-4f8459f26a

TaskStatus
java-jarsGitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64GitHub Actions
verify-rc-source-java-macos-amd64GitHub Actions

@vibhatha

Copy link
Copy Markdown
ContributorAuthor

@lidavidm the file noted in the error seems to be generated https://github.com/ursacomputing/crossbow/actions/runs/7622553317/job/20760815369#step:7:35295
Should we omit such generated resources from checkstyles?

@lidavidm

Copy link
Copy Markdown
Member

Yes

Comment threadjava/dev/checkstyle/checkstyle.xml Outdated
Comment on lines 226 to 233

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are these checks no longer supported?

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.

It showed up errors when running with those configs. I didn't log them, but I can reproduce the errors and post here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, let's just take a look and make sure - I don't care too much either way but perhaps some of these will still be useful even after google-java-format (since many of these seem more like lint things)

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.

Yes I agree, let me log one by one here.

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.

Listing compile errors for the removed attributes

  1. <property name="allowMissingThrowsTags" value="true"/>
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (validate) on project arrow-maven-plugins: Failed during checkstyle configuration: cannot initialize module TreeWalker - cannot initialize module JavadocMethod - Property 'allowMissingThrowsTags' does not exist, please check the documentation -> [Help 1]
  1. <property name="minLineCount" value="2"/>
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (validate) on project arrow-maven-plugins: Failed during checkstyle configuration: cannot initialize module TreeWalker - cannot initialize module JavadocMethod - Property 'minLineCount' does not exist, please check the documentation -> [Help 1]
  1. <property name="allowThrowsTagsForSubclasses" value="true"/>
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (validate) on project arrow-maven-plugins: Failed during checkstyle configuration: cannot initialize module TreeWalker - cannot initialize module JavadocMethod - Property 'allowThrowsTagsForSubclasses' does not exist, please check the documentation -> [Help 1]
  1. <property name="suppressLoadErrors" value="true"/>
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (validate) on project arrow-maven-plugins: Failed during checkstyle configuration: cannot initialize module TreeWalker - cannot initialize module JavadocMethod - Property 'suppressLoadErrors' does not exist, please check the documentation -> [Help 1]
  1. <property name="ignoreMethodNamesRegex" value="main"/>
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (validate) on project arrow-maven-plugins: Failed during checkstyle configuration: cannot initialize module TreeWalker - cannot initialize module JavadocMethod - Property 'ignoreMethodNamesRegex' does not exist, please check the documentation -> [Help 1]

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.

@lidavidm these are the errors I saw when each of these properties were used.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

https://github.com/checkstyle/checkstyle/issues/7329

  • allowMissingThrowsTags, allowThrowsTagsForSubclasses, suppressLoadErrors are now basically the default

https://github.com/checkstyle/checkstyle/issues/6703

  • minLineCount and ignoreMethodNamesRegex were moved to a new check called MissingJavadocMethodCheck, can you configure that to behave like our old one?

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.

Ah I see. Let me check.

@vibhathavibhathaFeb 5, 2024

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.

@vibhathavibhathaFeb 5, 2024

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.

allowMissingThrowsTags, allowThrowsTagsForSubclasses, suppressLoadErrors are now basically the default

These are not explicit properties in 8.29 but were in 8.19 but I couldn't find whether they are the defaults in 8.29.

@github-actionsgithub-actionsBot added awaiting merge Awaiting merge awaiting changes Awaiting changes and removed awaiting change review Awaiting change review awaiting merge Awaiting merge labels Jan 24, 2024
@vibhatha
vibhathaforce-pushed the feat-puppycrawl-update-8.29 branch from 0ded732 to ca838fbCompareFebruary 5, 2024 13:34
@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Feb 5, 2024
@github-actionsgithub-actionsBot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Feb 5, 2024
@vibhatha

Copy link
Copy Markdown
ContributorAuthor

@github-actions crossbow submit java

@github-actions

Copy link
Copy Markdown

Revision: ca838fb

Submitted crossbow builds: ursacomputing/crossbow @ actions-d6ad59a58d

TaskStatus
java-jarsGitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64GitHub Actions
verify-rc-source-java-macos-amd64GitHub Actions

@lidavidmlidavidm changed the title [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29MINOR: [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29Feb 5, 2024
@lidavidmlidavidm changed the title MINOR: [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29GH-39946: [Java] Bump com.puppycrawl.tools:checkstyle from 8.19 to 8.29Feb 5, 2024
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #39946has been automatically assigned in GitHub to PR creator.

@lidavidm
lidavidm merged commit cb5c109 into apache:mainFeb 5, 2024
@lidavidmlidavidm removed the awaiting merge Awaiting merge label Feb 5, 2024
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit cb5c109.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.

dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
… to 8.29 (apache#39694)
### Rationale for this change
This PR was created in place of apache#39202 to integrate the `puppycrawl.tools.checkstyle` upgrade. ### What changes are included in this PR?
Style changes in Java classes and core changes to the style format itself. Some unsupported attributes have been removed. And some attributes have
been reorganized upon the provided guidelines in the documentation. ### Are these changes tested?
N/A Tested by existing checkstyle guideline. ### Are there any user-facing changes?
No
* Closes: apache#39946
Lead-authored-by: Vibhatha Lakmal Abeykoon <vibhatha@gmail.com>
Co-authored-by: vibhatha <vibhatha@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
… to 8.29 (apache#39694)
### Rationale for this change
This PR was created in place of apache#39202 to integrate the `puppycrawl.tools.checkstyle` upgrade. ### What changes are included in this PR?
Style changes in Java classes and core changes to the style format itself. Some unsupported attributes have been removed. And some attributes have
been reorganized upon the provided guidelines in the documentation. ### Are these changes tested?
N/A Tested by existing checkstyle guideline. ### Are there any user-facing changes?
No
* Closes: apache#39946
Lead-authored-by: Vibhatha Lakmal Abeykoon <vibhatha@gmail.com>
Co-authored-by: vibhatha <vibhatha@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Java] Bump checkstyle to latest version supporting Java 8

2 participants

@vibhatha@lidavidm