4 ms·

A little discipline will go a long way here: - If the annotation is thought to be necessary because of a bug in the tool, add a link to the bug next to the dis

by l0b0 5y ago

A little discipline will go a long way here: - If the annotation is thought to be necessary because of a bug in the tool, add a link to the bug next to the disable comment. This makes it easy to get rid of the comment when being alerted that the bug is fixed. - If you happen to disagree with the tool in a specific instance, and there's no way to override just that case, mention why in the commit message. - Check for disable comments at peer review.

quietbritishjim 5y ago
Isn't it useful for the tool to warn about something that is often incorrect but sometimes intentional? The parent's example of implicit constructors is a good one, and a sibling comment mentioned fallthrough as another example. None of your suggestions help with that: - It's not a bug in the tool (because the warning is often useful). - A mention in a commit message doesn't help when you run the tool in future and get false alarms (oh I'll just read all the historic commit messages to see if any say these should be ignored). - If you find such a comment at peer review then that doesn't give you anything actionable to do if the comment is being used correctly in one of these casees. The solution in these cases is not "a little discipline" but simply to use the disabling comment and accept that this is reasonable.
- bluGill 5y ago
  > Isn't it useful for the tool to warn about something that is often incorrect but sometimes intentional? Only if the total number of warnings across your entire codebase is tiny - 10 is the [arbitrary] number I like to use. That is 10 warnings, it doesn't matter if you are writing 5 lines of hello world or a 100 million lines of a self driving car. When the number of ignored warnings get above that number people start to ignore the tool and then it becomes useless.
  - quietbritishjim 5y ago
    Your comment drives the discussion in a rather circular fashion: your objection is addressed by code comments that selectively disable warnings at specific locations in code, which is the subject of the original article. So long as you make careful use of those then it doesn't matter if you have 10,000 of them in your 100 million lines of code; you still only get one (visible) warning if you make a change that triggers it again, which is below your 10 warnings threshold.
    - bluGill 5y ago
      You need to regularly audit all your warnings with ever tool upgrade to make sure they still apply. Auditing 10000 issues is too hard, so people will soon not be careful. When there are 10 you can check each quickly enough and ensure they are still valid. With 10000 warnings that means that people have gotten in the habit of disabling warnings, so how many of them are real. I've tracked production bugs to someone disabling a warning - we regularly disabled that warning for correct reasons, but in the one case it wasn't a false warning. Thus your disabled warning levels need to be small. Small is somewhat arbitrary, but it needs to be small enough that when the tool says something is wrong you trust it.
    - TheCoelacanth 5y ago
      At that point, why not just make them errors? Making them warnings just makes it easier to put off the decision about whether to disable the rule for a specific case.

by l0b0 5y ago

quietbritishjim 5y ago

Isn't it useful for the tool to warn about something that is often incorrect but sometimes intentional? The parent's example of implicit constructors is a good one, and a sibling comment mentioned fallthrough as another example. None of your suggestions help with that: - It's not a bug in the tool (because the warning is often useful). - A mention in a commit message doesn't help when you run the tool in future and get false alarms (oh I'll just read all the historic commit messages to see if any say these should be ignored). - If you find such a comment at peer review then that doesn't give you anything actionable to do if the comment is being used correctly in one of these casees. The solution in these cases is not "a little discipline" but simply to use the disabling comment and accept that this is reasonable.

bluGill 5y ago
> Isn't it useful for the tool to warn about something that is often incorrect but sometimes intentional? Only if the total number of warnings across your entire codebase is tiny - 10 is the [arbitrary] number I like to use. That is 10 warnings, it doesn't matter if you are writing 5 lines of hello world or a 100 million lines of a self driving car. When the number of ignored warnings get above that number people start to ignore the tool and then it becomes useless.
- quietbritishjim 5y ago
  Your comment drives the discussion in a rather circular fashion: your objection is addressed by code comments that selectively disable warnings at specific locations in code, which is the subject of the original article. So long as you make careful use of those then it doesn't matter if you have 10,000 of them in your 100 million lines of code; you still only get one (visible) warning if you make a change that triggers it again, which is below your 10 warnings threshold.
  - bluGill 5y ago
    You need to regularly audit all your warnings with ever tool upgrade to make sure they still apply. Auditing 10000 issues is too hard, so people will soon not be careful. When there are 10 you can check each quickly enough and ensure they are still valid. With 10000 warnings that means that people have gotten in the habit of disabling warnings, so how many of them are real. I've tracked production bugs to someone disabling a warning - we regularly disabled that warning for correct reasons, but in the one case it wasn't a false warning. Thus your disabled warning levels need to be small. Small is somewhat arbitrary, but it needs to be small enough that when the tool says something is wrong you trust it.
  - TheCoelacanth 5y ago
    At that point, why not just make them errors? Making them warnings just makes it easier to put off the decision about whether to disable the rule for a specific case.