Skip to content

Issue #14194: Add LexicographicalAnnotationListing#17892

Merged
romani merged 1 commit into
checkstyle:masterfrom
Pankraz76:fix-error-prone-fix-maven
Oct 10, 2025
Merged

Issue #14194: Add LexicographicalAnnotationListing#17892
romani merged 1 commit into
checkstyle:masterfrom
Pankraz76:fix-error-prone-fix-maven

Conversation

@Pankraz76

Copy link
Copy Markdown

@Pankraz76 Pankraz76 force-pushed the fix-error-prone-fix-maven branch from 2e42eff to e3302cb Compare October 10, 2025 10:36
Comment thread pom.xml Outdated

@rickie rickie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Although sorting is good to do, we used the approach of enabling simply 1 check per pull request. We should probably do the same approach.

Additionally there should be some form of discussion on which rules to enable before simply doing it.

Comment thread pom.xml Outdated
-Xep:RedundantStringConversion:ERROR
-Xep:RedundantStringEscape:ERROR
-Xep:Slf4jLogStatement:ERROR
-Xep:StaticImport:ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can drop this if we in the line after simply turn it off again.

Comment thread pom.xml Outdated
Comment thread pom.xml Outdated
-Xep:MockitoMockClassReference:ERROR
-Xep:MockitoStubbing:ERROR
-Xep:NestedOptionals:ERROR
-Xep:NonEmptyMono:ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NonEmptyMono is a check for Reactor code, that's not necessary for this codebase.

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.

we found it conveninent to reference all validaiton modules that tool provides and just doe OFF for some rules and having comment above to explain why not use it.
in this case even in few years we can check list of used rules against tool documentation and see what is missed to be activated and what is deactivated for a reason.

Comment thread pom.xml Outdated
@Pankraz76 Pankraz76 force-pushed the fix-error-prone-fix-maven branch from e3302cb to 6022860 Compare October 10, 2025 12:24
@Pankraz76 Pankraz76 requested a review from rickie October 10, 2025 12:25
@Pankraz76 Pankraz76 marked this pull request as ready for review October 10, 2025 12:26

@rickie rickie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀

@romani

romani commented Oct 10, 2025

Copy link
Copy Markdown
Member

@Pankraz76 , please fix other violations and we good to merge.

@Pankraz76 Pankraz76 force-pushed the fix-error-prone-fix-maven branch from 6022860 to 890d488 Compare October 10, 2025 14:56
@Pankraz76 Pankraz76 force-pushed the fix-error-prone-fix-maven branch from 890d488 to 334aba8 Compare October 10, 2025 14:56
@Pankraz76

Copy link
Copy Markdown
Author

We really need to address the patching process afterward to avoid the added price for this feature. We should improve acceptance through automation.

@Pankraz76 Pankraz76 requested a review from romani October 10, 2025 15:00
@Pankraz76

Copy link
Copy Markdown
Author

thanks for the quick-win.

@romani romani left a comment

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.

Thanks a lot for keeping PR small and single point of fix, it helps a lot to accept it quickly

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.

3 participants