3 ms·
This works better as "All tests decreasing the overall percentage of tested code must have a good explanation signed off on by the reviewer". I've occasionally
by strken 3y ago
This works better as "All tests decreasing the overall percentage of tested code must have a good explanation signed off on by the reviewer". I've occasionally deleted dead code paths, had an almost entirely red diff, had close to 100% test coverage over the functions I touched, and yet decreased the overall percentage of tested code because the dead code path was more heavily tested than the rest of the code base.
- stavros 3y agoI kind of disagree, in that any rule should have an implied "you can ignore this with good justification". Otherwise you turn people into robots, which renders moot all the reasons why you have people, and not robots.
- strken 3y agoI agree with this - in practice any rule can be broken - but how easily it's broken and whether developers are explicitly told to break it is a process decision. How big a roadblock is your CI putting in the path of developers? If you require e.g. sign-off from a team lead or manager or whoever has "merge anyway" permissions in GitHub, that's more difficult than sign-off from the same reviewer who's reading your code anyway. You can make it easier or harder to break rules and in this case I think it should be so easy to break that there's a codified procedure to do so.
- danielheath 3y agoI haven’t tried that - in that situation we did “find something that needs a test”