What problem does this solve?
This repository's own parse-coverage report can get worse without anyone noticing. Nothing
in CI reads it, so a change that widens a parse-error range, or drops a region from the
index, lands green. The coverage work tracked in #963 improved the report itself, but the
report is only useful if a regression in it is caught at the time it happens rather than
months later.
I hit this while working on the report: I could not tell whether a number I was looking at
was a regression or had always been that way, because no run had ever recorded a baseline.
Proposed solution
Add a CI check that reads this repository's own parse-coverage report and fails a pull
request when it goes backwards against a recorded ceiling.
The reason this needs a decision from you rather than a pull request is that it adds a
required check to pr-smoke, and CONTRIBUTING.md line 128 puts that class of change
behind a design discussion. There are real trade-offs:
- A ceiling has to move when
main legitimately changes. It already did once: the ceiling
rose from 58 to 59 because main added src/daemon/runtime.c:47, whose function-style
_Atomic(type) the C grammar cannot parse. That rise came from main, not from the
branch under test, and a contributor hitting it would have no way to tell.
- A required check that can fail for reasons outside the contributor's change is a tax on
every pull request, not only on the ones that touch coverage.
So the question is whether you want the gate at all, and if so whether it should be
required or advisory to begin with.
Alternatives considered
- Advisory only. Report the number on each pull request without failing. Catches the
regression in review rather than automatically, and costs nothing when main shifts.
- Nightly rather than per-PR. Removes the tax from pull requests entirely and still
catches a regression within a day, at the cost of not naming the change that caused it.
- Do nothing. The report stays useful for a person who reads it, and silently drifts
otherwise.
I have an implementation ready either way — it is currently open as a pull request, split
out of the parse-coverage fix at review request precisely because it is a maintainer
decision rather than part of a bug fix. Happy to close that and keep only whichever shape
you prefer, or none.
Confirmations
What problem does this solve?
This repository's own parse-coverage report can get worse without anyone noticing. Nothing
in CI reads it, so a change that widens a parse-error range, or drops a region from the
index, lands green. The coverage work tracked in #963 improved the report itself, but the
report is only useful if a regression in it is caught at the time it happens rather than
months later.
I hit this while working on the report: I could not tell whether a number I was looking at
was a regression or had always been that way, because no run had ever recorded a baseline.
Proposed solution
Add a CI check that reads this repository's own parse-coverage report and fails a pull
request when it goes backwards against a recorded ceiling.
The reason this needs a decision from you rather than a pull request is that it adds a
required check to
pr-smoke, andCONTRIBUTING.mdline 128 puts that class of changebehind a design discussion. There are real trade-offs:
mainlegitimately changes. It already did once: the ceilingrose from 58 to 59 because
mainaddedsrc/daemon/runtime.c:47, whose function-style_Atomic(type)the C grammar cannot parse. That rise came frommain, not from thebranch under test, and a contributor hitting it would have no way to tell.
every pull request, not only on the ones that touch coverage.
So the question is whether you want the gate at all, and if so whether it should be
required or advisory to begin with.
Alternatives considered
regression in review rather than automatically, and costs nothing when
mainshifts.catches a regression within a day, at the cost of not naming the change that caused it.
otherwise.
I have an implementation ready either way — it is currently open as a pull request, split
out of the parse-coverage fix at review request precisely because it is a maintainer
decision rather than part of a bug fix. Happy to close that and keep only whichever shape
you prefer, or none.
Confirmations