Repository navigation
stubtest: should unused allowlist entries be an error in stdlib? #6406
Description
In third-party stubs, unused allowlist entries cause CI to fail, and must be removed in the same pull request that fixes whatever causes it to be unused.
- Advantage: super simple
- Advantage: you can be sure you fixed an allowlist entry when you make a pull request
- Disadvantage: can be confusing for new contributors
- Disadvantage: can cause merge conflicts if one pull request removes allowlist entry from line 17 and another removes one from line 18 (I don't think this has ever happened)
In stdlib, unused allowlist entries don't cause CI errors, and are removed in a weeky automatically generated pull request.
- Advantage: new contributors don't need to worry about the allowlists (more important for stdlib, because there's many allowlists)
- Advantage: no merge conflicts
- Disadvantage: complicated
- Disadvantage: automatic removal doesn't work in all corner cases
- Disadvantage: automatic removal can fail without anyone noticing it (unlike a CI failure of a pull request)
I like how this works for third-party stubs. Should we do the same for stdlib too?
Activity
hauntsaninja commentedon Nov 28, 2021
Yeah, I've actually found "this didn't get fully fixed but we didn't realise" to be somewhat common, e.g. from today I found that #5388 missed a default arg that I'm fixing in #6417. I've definitely made several mistakes in this vein.
I think my personal standpoint is that as we get to the point where most things in the allowlists are "can't / won't fix", on net erroring for unused allowlist becomes more valuable and less disruptive. And we've already made so much progress towards this!