Introduce parameter failOnMismatchedAllowedViolationsCount #165
Conversation
…e build when the max number of violations is not equal to the actual number of violations but is less
elharo
left a comment
elharo
left a comment
There was a problem hiding this comment.
I think a new user visible parameter is significant enough to require a JIRA issue.
We have switched to GitHub Issues .... with this project 😄 |
|
Github issue then. You are correct not all PRs require issues. However we should have an issue for user visible functionality changes for the release notes. |
I created #174 |
|
I also noticed that the template still says "Make sure there is a JIRA issue filed". We should probably fix that (not in this PR of course) |
As we have wrote at https://maven.apache.org/developers/conventions/github.html |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new configuration parameter failOnMismatchedAllowedViolationsCount that enforces exact matching of checkstyle violations to the configured maxAllowedViolations threshold. This prevents scenarios where violations decrease but the threshold isn't updated, leaving room for new violations to be introduced without detection.
Key Changes:
- Added
failOnMismatchedAllowedViolationsCountparameter (default: false) to fail builds when violation count doesn't equalmaxAllowedViolations - Implemented logic to check for mismatched violation counts when violations are at or below the configured limit
- Added integration test to verify the new feature works correctly
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/apache/maven/plugins/checkstyle/CheckstyleViolationCheckMojo.java | Adds new boolean parameter with JavaDoc documentation and implements the mismatch check logic in the execute method |
| src/it/check-fail-with-mismatch-violation-count/verify.groovy | Integration test verification script that asserts the expected failure message appears in build log |
| src/it/check-fail-with-mismatch-violation-count/src/main/java/org/package-info.java | Test Java file providing package documentation for checkstyle validation |
| src/it/check-fail-with-mismatch-violation-count/src/main/java/org/MyClass.java | Test Java class with intentional checkstyle violation for integration testing |
| src/it/check-fail-with-mismatch-violation-count/pom.xml | Test project configuration with new parameter enabled and maxAllowedViolations set to 314 (mismatched with actual count of 1) |
| src/it/check-fail-with-mismatch-violation-count/invoker.properties | Invoker configuration specifying the test should result in a build failure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * <li>some other changes introduces new violations but the build does not fail, as the count left some margin for error</li> | ||
| * </ul> | ||
| * | ||
| * * @since 3.6.1 |
There was a problem hiding this comment.
There's a formatting error in the JavaDoc comment. The @since tag has an extra asterisk before it, making it * @since instead of @since.
| * * @since 3.6.1 | |
| * @since 3.6.1 |
| * <p>This prevents the following scenario: | ||
| * <ul> | ||
| * <li>some changes reduce the checkstyle count but maxAllowedViolations is not updated</li> | ||
| * <li>some other changes introduces new violations but the build does not fail, as the count left some margin for error</li> |
There was a problem hiding this comment.
Grammatical error: "introduces" should be "introduce" since the subject "changes" is plural.
| * <li>some other changes introduces new violations but the build does not fail, as the count left some margin for error</li> | |
| * <li>some other changes introduce new violations but the build does not fail, as the count left some margin for error</li> |
3 participants
This parameter allows to fail the build when the the actual number of violations is less than the max number of violations.
fixes #174
I did not open a Jira account as the change is small (17 lines without the tests)
Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[MCHECKSTYLE-XXX] - Fixes bug in ApproximateQuantiles,where you replace
MCHECKSTYLE-XXXwith the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean verifyto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
mvn -Prun-its clean verify).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.