Description
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:
checkstyle/config/checkstyle-input-suppressions.xml
Lines 68 to 70 in b6301c1
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: f792773