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

Remove suppressions for original verify method or explain in comment #10390

Closed
timurt opened this issue Jul 19, 2021 · 4 comments
Closed

Remove suppressions for original verify method or explain in comment #10390

timurt opened this issue Jul 19, 2021 · 4 comments

Comments

@timurt
Copy link
Contributor

timurt commented Jul 19, 2021

Before:
#10391
#10081

Scope of this issue:

  • Update test classes one by one (one PR for each test)
  • Migrate to new verify method
  • Make sure code coverage and pitest coverage didn't change
  • Remove suppressions for original verify method or expalin each/group of suppression why they stay permanent. All tests on fake/test Checks should be suppressed permanently, as there is no business value in such tests and they exists only for coverage of edge cases that are not achievable by existing public Checks.

Suppression:

<!-- untill https://github.com/checkstyle/checkstyle/issues/10390 -->

we need clear explanation on why each suppresion is required. If we are on lack of time to migrate, but there is no good reason to stay in permanent, separate issue should be create and placed as comment <!-- until LINK-->

shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Aug 25, 2021
@romani romani added this to the 9.0 milestone Aug 25, 2021
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Aug 25, 2021
shashwatj07 added a commit to shashwatj07/checkstyle that referenced this issue Aug 25, 2021
@romani romani changed the title Migrate to new verify method for inlined configs Remove suppressions for original verify method or explain in comment Dec 21, 2021
@rnveach
Copy link
Member

rnveach commented Mar 12, 2022

@romani @timurt Is this issue still open and what is left to do? I could not find suppression listed in first post.

@romani
Copy link
Member

romani commented Mar 12, 2022

Link is working fine, bunch of suppression still there, we need to cleanup/explain them or fix them.

@romani
Copy link
Member

romani commented Mar 12, 2022

Yes, we claim now that all is resolved

<!-- Inline Config cannot be supported. -->

But I would rather recheck why list is so big.
There might be some new separate tickets need to be created to resolve some of them.

See PR below, we might need check onemore time all excludes to prove reason that we cannot use new api (and write a reason in comment over exlude) or do final update.

@romani
Copy link
Member

romani commented Mar 23, 2022

I am closing this issue, we will do final updates at #11446

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

3 participants