You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@storm.apache.org by "GGraziadei (via GitHub)" <gi...@apache.org> on 2026/05/05 19:20:08 UTC
[I] [STORM-XXXX] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
GGraziadei opened a new issue, #8585:
URL: https://github.com/apache/storm/issues/8585
The project is currently using a significantly outdated version of Checkstyle (v8.2).
This version is several years old and lacks support for modern Java syntax and improved linting features found in the latest releases.
https://mvnrepository.com/artifact/com.puppycrawl.tools/checkstyle/versions
Continuing with this outdated version presents several issues:
- Strict import enforcement: Newer versions of Checkstyle handle CustomImportOrder differently. We are currently seeing warnings (or being forced into specific import hacks) regarding the STATIC group because the old version's rules are inconsistent with modern IDE defaults.
- Configuration inconsistency: the file `storm_checkstyle.xml` is based on an ancient version of Google’s checkstyle config (v7.7). Many properties used in our XML are deprecated or have been removed in Checkstyle 10+, making the current configuration incompatible with the latest tooling.
- Modern Java support.
Proposed Changes:
- Bump dependency
- Update `storm_checkstyle.xml`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "sigee (via GitHub)" <gi...@apache.org>.
sigee commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-4472473373
Hi @GGraziadei, @rzo1,
I'd like to suggest a phased migration strategy that addresses the reviewability concern while still making progress:
**Phase 1: Bump the Checkstyle plugin version only**
Upgrade the plugin to the target version (e.g., 12.3.1) while keeping `storm_checkstyle.xml` completely unchanged. This isolates any hard breakage — rules that have been renamed or removed in Checkstyle 10+ — into a single, small PR. No code files are touched, so it's easy to review.
**Phase 2: Modernize `storm_checkstyle.xml` with `severity="warning"`**
Introduce new and updated rules into the config, but set them to `severity="warning"` rather than `"error"`. The build stays green, nothing regresses, and reviewers only see config changes — not a flood of code fixes mixed in.
**Phase 3: Fix warnings incrementally**
Address the warnings module by module (or I would say rule by rule), exactly as suggested above. Each PR remains small and reviewable.
The core idea is to decouple the *tooling upgrade*, the *rule configuration*, and the *code fixes* into three clearly separated concerns. This keeps every step reviewable and avoids the "touches almost every file" problem that makes both Strategy 1 and Strategy 2 hard to justify as a single effort.
One caveat for Phase 1: a small number of config fixups may still be needed if any currently-used rules have been removed or renamed in the new version, but that should be minimal and scoped entirely to `storm_checkstyle.xml`.
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [STORM-XXXX] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-4382329669
This can be done, but it might require to go module by module - otherwise, potential checkstyle violations and their fixes become unreviewable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "GGraziadei (via GitHub)" <gi...@apache.org>.
GGraziadei commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-4395363892
Hi @rzo1,
I have explored several paths for this update. First, I found the targeting version 12.3.1 (according to the compatibility matrix), as it is the last checkstyle version developed with Java 17, ensuring maximum compatibility with the maven-checkstyle-plugin.
I’ve evaluated two main strategies:
Strategy 1: Update the ceckstyle configuration. This involves adopting updated rules (e.g., [Google Checks 12.3.1](https://github.com/checkstyle/checkstyle/blob/checkstyle-12.3.1/src/main/resources/google_checks.xml)) and skipping auto-generated code. Pros: Up-to-date standards. Cons: This would require touching almost every file in the project, which may not provide enough benefit to justify the effort.
Strategy 2: Keep custom rules but fix mandatory violations. This would involve keeping our current rules while fixing only the mandatory violations introduced by the new version. Even when using tools like OpenRewrite https://docs.openrewrite.org/ to automate the process, a large percentage of files would still need modifications.
In both cases, the cost-benefit analysis leans negative. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-4477005166
That seems fine to me.
@Richard Zowalla ***@***.***> any thoughts?
On Sun, 17 May 2026 at 21:52, Dávid Szigecsán ***@***.***>
wrote:
> *sigee* left a comment (apache/storm#8585)
> <https://github.com/apache/storm/issues/8585#issuecomment-4472473373>
>
> Hi @GGraziadei <https://github.com/GGraziadei>, @rzo1
> <https://github.com/rzo1>,
>
> I'd like to suggest a phased migration strategy that addresses the
> reviewability concern while still making progress:
>
> *Phase 1: Bump the Checkstyle plugin version only*
> Upgrade the plugin to the target version (e.g., 12.3.1) while keeping
> storm_checkstyle.xml completely unchanged. This isolates any hard
> breakage — rules that have been renamed or removed in Checkstyle 10+ — into
> a single, small PR. No code files are touched, so it's easy to review.
>
> *Phase 2: Modernize storm_checkstyle.xml with severity="warning"*
> Introduce new and updated rules into the config, but set them to
> severity="warning" rather than "error". The build stays green, nothing
> regresses, and reviewers only see config changes — not a flood of code
> fixes mixed in.
>
> *Phase 3: Fix warnings incrementally*
> Address the warnings module by module (or I would say rule by rule),
> exactly as suggested above. Each PR remains small and reviewable.
>
> The core idea is to decouple the *tooling upgrade*, the *rule
> configuration*, and the *code fixes* into three clearly separated
> concerns. This keeps every step reviewable and avoids the "touches almost
> every file" problem that makes both Strategy 1 and Strategy 2 hard to
> justify as a single effort.
>
> One caveat for Phase 1: a small number of config fixups may still be
> needed if any currently-used rules have been removed or renamed in the new
> version, but that should be minimal and scoped entirely to
> storm_checkstyle.xml.
>
> What do you think?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/storm/issues/8585#issuecomment-4472473373>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAG5GIQODDYRMUJYQI6DYML43IRBTAVCNFSM6AAAAACYSBWQLKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DINZSGQ3TGMZXGM>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "reiabreu (via GitHub)" <gi...@apache.org>.
reiabreu commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-5185341311
mentioned by https://github.com/apache/storm/pull/8969/changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-4477243127
Plan Sounds good
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-4395396860
Storm 3 will Most likely target Java 21 (as discussed on dev).
Yes, migrating is a lot of mechanical work. If someone wants to do it, I am fine with 1 or 2.
Only thing is, that it needs to be done step by step. Otherwise, reviewing is Not feasable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-4477242096
Feel free to help @sigee :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Upgrade Checkstyle to latest version and modernize `storm_checkstyle.xml` (storm)
Posted by "GGraziadei (via GitHub)" <gi...@apache.org>.
GGraziadei commented on issue #8585:
URL: https://github.com/apache/storm/issues/8585#issuecomment-6253878775
Aut-fix tooling merged with #9017
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org