verible icon indicating copy to clipboard operation
verible copied to clipboard

line length rule: add a parameter to check comments

Open wsipak opened this issue 3 years ago • 3 comments

This addresses https://github.com/chipsalliance/verible/issues/941 There's a new parameter ~~allow_comments~~ ignore_comments. When it's set to false, exceeding the line length limit in comments won't be forgiven. The default value is true, so that it doesn't change the way it works by default now. It works with single line comments and block comments, but those are not distinguished in configuration right now (Should we consider them separately?).

Also, would it be preferred to rename the parameter to something like check_comments so that it has the opposite meaning, and set to false would result in not linting comments?

Would it be better to have two separate values of length limit for non-comment and comment lines? This probably could be addressed and implemented in another PR, but I'm noting it here as a thing to consider.

wsipak avatar Oct 06 '21 14:10 wsipak

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (de4211b) 93.13% compared to head (5ee7eeb) 93.14%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #972   +/-   ##
=======================================
  Coverage   93.13%   93.14%           
=======================================
  Files         355      355           
  Lines       25843    25882   +39     
=======================================
+ Hits        24069    24107   +38     
- Misses       1774     1775    +1     
Impacted Files Coverage Δ
verilog/analysis/checkers/line_length_rule.h 100.00% <ø> (ø)
verilog/analysis/checkers/line_length_rule.cc 97.33% <100.00%> (-0.63%) :arrow_down:

... and 2 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Oct 06 '21 15:10 codecov-commenter

Yes, I think for the user's point of view a parameter check_comments would be easiest to understand.

I tihnk diffentiating between end-of-line and block comments w.r.t warning or not is probably too much user-confusing option than worth it (and I wouldn't see why it should make a difference).

Same for different length of comment vs. non-comment. All of them should stay within the same line length.

hzeller avatar Oct 06 '21 20:10 hzeller

@hzeller Thank you for your feedback. The parameter is now named check_comments.

wsipak avatar Oct 07 '21 13:10 wsipak