Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #6981: IT regression area input files Suppressed & Pattern validation updated to "InputXpath{Checkname}Xxx.java" #14713

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

MANISH-K-07
Copy link
Contributor

@MANISH-K-07 MANISH-K-07 commented Mar 24, 2024

Aims to close #6981

PR not ready for review !!
I was facing issues with mvn validations on local. So, please excuse me while I use our integration workflows for verifying

Update : The pattern has been updated at XpathRegressionTest#validateInputDirectory to validate "InputXpath{Checkname}Xxxx.java". All existing old named modules have been suppressed.

<--Unrelated-->
Fun fact : Our IDEs have this keyboard shortcut ctrl+shift+z to single-line comment the line on which cursor is 馃槃

@MANISH-K-07 MANISH-K-07 force-pushed the InputXpath branch 3 times, most recently from c0b0db6 to de281f8 Compare March 24, 2024 07:25
@MANISH-K-07 MANISH-K-07 marked this pull request as ready for review March 24, 2024 08:16
@MANISH-K-07
Copy link
Contributor Author

@romani ,@rnveach @rnveach Please see if this matches your expectations :)

@@ -117,6 +117,149 @@ public class XpathRegressionTest extends AbstractModuleTestSupport {

private static final Set<String> INTERNAL_MODULES = getInternalModules();

// Checks whose files need to be renamed to new pattern "InputXpath{Check}Xxx.java"
// till checkstyle issue
private static final Set<String> RENAME_INPUT_XPATH = Set.of(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are then good to completely swap old enforcement with new as it will be fixed as your list is worked on. The list must reference the issue to work on (until https://....) and complete it. I recommend to make this a new issue, we can mark it as easy

After your confirmation, I will create a new issue as suggested at #14595 (comment) and update comment above

Copy link
Member

@rnveach rnveach left a comment

Choose a reason for hiding this comment

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

minor: Renamed IT executablestatementcount to new Pattern

This shouldn't be minor. Either connect it to this issue, or make it supplemental of this issue.

Also we need the new issue number in the code before this can be merged.

I am fine with everything else and will assign the admins.

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

Ok to merge

@MANISH-K-07 MANISH-K-07 reopened this Mar 24, 2024
@MANISH-K-07
Copy link
Contributor Author

Ok to merge

Sorry I closed and reopened by mistake..
I have attached the suppression list at Issue #14715

Copy link
Member

@nrmancuso nrmancuso left a comment

Choose a reason for hiding this comment

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

Let's go!

@nrmancuso nrmancuso merged commit e70b8b5 into checkstyle:master Mar 26, 2024
156 of 159 checks passed
@MANISH-K-07 MANISH-K-07 deleted the InputXpath branch March 26, 2024 01:48
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.

Enforce input naming convention in IT
4 participants