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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: fail checkstyle on all findings except warning.LOW #4899

Merged
merged 17 commits into from
Nov 27, 2021

Conversation

jdrueckert
Copy link
Member

Currently, the analysis stage only fails for findings of severity "error".
Findings of severities "warning.HIGH", "warning.NORMAL" and "warning.LOW" do not result in a failure.
This PR attempts to reduce the acceptable severity to "warning.LOW" only, all findings of higher severities should fail the build.

@jdrueckert jdrueckert added Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Category: Test/QA Requests, Issues and Changes targeting tests and quality assurance Size: S Small effort likely only affecting a single area and requiring little to no research Type: Chore Request for or implementation of maintenance changes labels Sep 9, 2021
@jdrueckert jdrueckert self-assigned this Sep 9, 2021
@jdrueckert
Copy link
Member Author

Meh, that somehow does not work as expected 🤔

@pollend
Copy link
Member

pollend commented Sep 11, 2021

I think I fixed the syntax errors @jdrueckert

* <li><a href="https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/application-verifier">Application Verifier
* (<code>AppVerif.exe</code>)</a>, available from the Windows SDK</li>
* <li>
* <a href="https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/application-verifier">Application Verifier (<code>AppVerif.exe</code>)</a>,
Copy link
Member

Choose a reason for hiding this comment

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

any clue about getting rid of this LineLenghtCheck. sometimes its better to format this with one long line.

I think this is a lot more readable then trying to work out how to chop the line to the right length.

     * <ul>
     *     <li><a href="https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/application-verifier">Application Verifier (<code>AppVerif.exe</code>)</a>,available from the Windows SDK</li>
     *     <li><a href="https://github.com/lowleveldesign/process-governor">Process Governor (<code>procgov</code>)</a>,an open source third-party tool</li>
     * </ul>

Copy link
Member Author

Choose a reason for hiding this comment

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

As @skaldarnar mentioned on Discord, for these "sometimes" cases there's annotations that allow exactly that: ignoring the check rule in a particular case.

@pollend
Copy link
Member

pollend commented Sep 12, 2021

maybe we can adjust the checkstyle warnings: https://checkstyle.sourceforge.io/property_types.html#SeverityLevel

@jdrueckert
Copy link
Member Author

@jdrueckert jdrueckert merged commit 32761bf into develop Nov 27, 2021
@jdrueckert jdrueckert deleted the ci/fail-checkstyle-on-warnings branch November 27, 2021 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Build/CI Requests, Issues and Changes targeting gradle, groovy, Jenkins, etc. Category: Test/QA Requests, Issues and Changes targeting tests and quality assurance Size: S Small effort likely only affecting a single area and requiring little to no research Type: Chore Request for or implementation of maintenance changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants