You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I think its pretty unavoidable that for non-stylistic cops, there will be >0 false positives. In such instances we can disable the cop, and so long as we aren't regularly disabling the cop instead of fixing the issue, this isn't a bad thing.
I believe its commonly consider best practice to justify the disabling of a cop, but AFAIK there is no programmatic way to enforce this. Justifications might not always be good of course, but by forcing it to be written down we can increase the odds that a peer reviewer identifies the flawed logic.
Has such functionality been discussed / implemented before? I could not find evidence of that, but please
do share them if you are.
Failing that, I propose introducing a formal syntax for justifying, and a cop to enforce the presence of a justification
# RUBOCOP: This batch is too big to run validations, and there are none on this attributerelation.in_batches.update_all(verified: true)# rubocop:disable Rails/SkipsModelValidations
# rubocop:disable:next Rails/SkipsModelValidations -- This batch is too big to run validations, # and there are none on this attributerelation.in_batches.update_all(verified: true)
For existing codebases, there would need to be a way to note the absence of an existing justification when the cop was enabled, e.g.
# RUBOCOP: Disabled before Lint/JustifyDisable was enabledrelation.in_batches.update_all(verified: true)# rubocop:disable Rails/SkipsModelValidations
or
# rubocop:disable:next Rails/SkipsModelValidations -- Disabled before Lint/JustifyDisable was enabledrelation.in_batches.update_all(verified: true)
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
I think its pretty unavoidable that for non-stylistic cops, there will be >0 false positives. In such instances we can disable the cop, and so long as we aren't regularly disabling the cop instead of fixing the issue, this isn't a bad thing.
I believe its commonly consider best practice to justify the disabling of a cop, but AFAIK there is no programmatic way to enforce this. Justifications might not always be good of course, but by forcing it to be written down we can increase the odds that a peer reviewer identifies the flawed logic.
Has such functionality been discussed / implemented before? I could not find evidence of that, but please
do share them if you are.
Failing that, I propose introducing a formal syntax for justifying, and a cop to enforce the presence of a justification
Or a slightly nicer syntax (that relies on a non-existent Rubocop feature) might be
For existing codebases, there would need to be a way to note the absence of an existing justification when the cop was enabled, e.g.
or
Beta Was this translation helpful? Give feedback.
All reactions