Remove '//ok' comments from Input files

[romani]

After hundreds of example PR reviews, I , and not only I found that comment ok is just a noise that makes it hard to read. This example is small, so you will not fill pain, but, there are more complicated examples and it is not good. I agree to put comment to explain some not obvious reason, but it will be exception.

For now I treat all //ok comments as stuff to remove, there might be cases to keep them as form of: // ok, here is not obvious reason // ok, here is pointing to Check limitation In short - when it is not obvious.

all suppression <suppress id="UnnecessaryOkComment" for at https://github.com/checkstyle/checkstyle/blob/master/config/checkstyle-input-suppressions.xml should be removed and referenced files updated to not have "// ok" comments

We do need this removals in test inputs. Same is for Example files, it just makes example full of content that is hard to distinguish without code coloring.

user should see is comment as red flag of attention. All other lines are without attention, as no violation on them, on all of them.

We use to have such comments only at times when we had no enforcing and it was very manual markers , but it lost its role after we finished Input based tests

Such ok comment become to appear in completely unrelated places, by copy paste, and keep it at right place is not possible( or hard and outside of this project). Better without it . Location of violation comment is enforced by runtime - never outdated.

Please choose name of module from file: https://github.com/checkstyle/checkstyle/blob/b6301c189ef5d2d731a755fa9a2e84aa0677494a/config/checkstyle-input-suppressions.xml#L68-L70 where name of module is equalsavoidnull and send PR for all files that belong to it in single PR.

final goal is to remove all suppressions of "UnnecessaryOkComment".

Example of update: https://github.com/checkstyle/checkstyle/pull/13609/files attention on removal of suppression See more examples below for "Merged" Pull Requests.

---

One more example of expected update: https://github.com/checkstyle/checkstyle/commit/f7927735cc28b62d7b40e46a944c7d495a531f68

[romani]

@Vyom-Yadav , @Kevin222004 , @rdiachenko , @nrmancuso , please share your opinion on this issue.

[nrmancuso]

Agree 100%, I think overuse of comments diminishes the concision of examples and inputs.

[Vyom-Yadav]

Agreed, let's only permit // violation and // ok, because of ........

[Kevin222004]

I also agree to remove // ok.

[rdiachenko]

user should see is comment as red flag of attention. All other lines are without attention, as no violation on them, on all of them.

+1

[romani]

@stoyanK7 , please share your position on this

[stoyanK7]

I also agree with removing // ok

[romani]

@rnveach , please share your thoughts

[romani]

@sheetalj2205, we are ready to start removal. Referenced PR is merged.

[sheetalj2205]

@sheetalj2205, we are ready to start removal. Referenced PR is merged.

Okay, I am on it.

[sheetalj2205]

Hello @romani I just want to check before moving forward whether I understood it correctly or not, as we can see in these files InputDefaultComesLastDefaultMethodsInInterface.java and illegaltype/InputIllegalTypeEmptyStringMemberModifiers.java there are multiple //ok comments, we need to remove that and put it only at one place that is after class or interface name and I am supposed to do that for all the files of com.puppycrawl.tools.checkstyle.checks.coding package.

[romani]

We need to remove all //ok comments and keep only comments that some text after it like //ok bla bla bla

[sheetalj2205]

We need to remove all //ok comments and keep only comments that some text after it like //ok bla bla bla

can you please assign this to me

[nrmancuso]

@sheetalj2205 we do not assign issues except to maintainers, just make a comment like “I am on XXXCheck” and send a PR.

[romani]

@sheetalj2205, just make comment "I am xxxxxCheck tests" and send PR for single Check only. After we merge first PR you can send PRs for more, by group. We do not allow huge PRs that change all, such PRs usually never come to merge state.

[nrmancuso]

I am on MethodParamPadCheck

[suniti0804]

I am on covariantequals

[suniti0804]

I am on defaultcomeslast

[dontwike]

Please can you assign this issue.

[romani]

No assignments, just make a comment what check/module you are going to work and keep sending PRs. Thanks a lot for helping.

[dontwike]

Okay

[harshit4311]

I would like to work on this issue!

[romani]

Just start sending PRs.

[Zopsss]

I'm on InputNoEnumTrailingComma

[TarunAlapati-git]

I am on missing switchdefault

[mahfouz72]

Iam on unusedlocalvariable

[SinghaAnirban005]

I am on operatorwrap

[SinghaAnirban005]

I am on InputNoFinalizerFallThroughJava

[Ishantgarg-web]

I am on InputOuterTypeFilename2

[SinghaAnirban005]

I am on separatorwrap

[FloofCat]

I'm on InputEmptyCatchBlockViolationsByComment

[anishlukk123]

im on InputOuterTypeFilename2.java without cr from Ishantgarg-web

[MANISH-K-07]

Hey! I am on equalsavoidnull.

[MANISH-K-07]

I am on InputOuterTypeFilename.

[MANISH-K-07]

I am on 'illegaltype'

[RatikaDogra]

Hii.. I want to solve this issue please assign me

[romani]

No assignments, just make a comment what Test you updating and send PR.

[Anirban6756]

Remove '//ok' comments from the input files

[MANISH-K-07]

I am working on requirethis.

[sktpy]

I am on SuppressWarningsCheck

[RatikaDogra]

Remove '//ok' comments from the input files

[ankitshokeen]

removed '// ok' comments from input files #14325 https://github.com/checkstyle/checkstyle/pull/14325/files

[RatikaDogra]

Removing //ok from abbreviationaswordinname under naming directory

[ViswanathBalla22]

hi @romani @nrmancuso I want to write the code that user will not able to write only\ok comment this helps us to next time we not get new ok comment my aproach for this is create java file name as CustomOkcomment check that extends AbstarctCheck ,this code for user to avoid using simple //ok comments and they need to provide more context or remove if unnecessary. and the custom check to checksyle.xml, create a new module

[romani]

I want to write the code that user will not able to write only\ok comment this helps us to next time we not get new ok comment

We already have this validation, in this issue we just resolving old problems, that is why removal of suppression lines is required in each update

[ratatall]

Hey! Just wanted to notify you that I'm working on InputUnnecessaryParenthesesIfStatement2 to remove all ok comments.

[555vedant]

@romani sir , as you mentioned in above issue , we have to remove this from each individual files ?

[MANISH-K-07]

@romani sir , as you mentioned in above issue , we have to remove this from each individual files ?

@555vedant Pick a module, comment "I am on xxx" and remove all related suppressions. It is recommended to limit your PR to single module

[HRuii1]

Hi I'm on InputRequireThisTryWithResources

[abhishekmaity]

I am on InputRequireThisTryWithResourcesOnlyOverlappingFalse