REQUIREMENTS_MAPPING.md maps Two Party Review to REVIEW_ENFORCED, which checks that a
repository requires pull requests, an approval, dismissal of stale approvals, code-owner
review, and approval of the most recent reviewable push.
Those are properties of the repository's configuration at the moment the control runs.
The requirement they map to is a property of a revision:
[Final revision approved] This requirement applies to the final revision
submitted. I.e. if additional changes are made during the review process, those
changes MUST be reviewed as well.
The two come apart in at least two places, and I would rather ask whether you consider
them in scope than assume either way.
A merge whose result nobody saw. "Require branches to be up to date before merging"
is a separate setting and is commonly off, because turning it on serialises a busy repo.
With it off, a pull request approved against base A merges into base B, and the tree that
lands is one no reviewer was shown. Every REVIEW_ENFORCED check still passes: there was
a PR, there was an approval, no commit was pushed after it. The approval was real and it
covered a different tree.
The control is evaluated now; the revision was merged then. A repository that
relaxed a ruleset for an afternoon, or set these rules up last month, reports the same
REVIEW_ENFORCED today as one that always had them. The configuration is current state;
the history is not covered by it.
Neither is a defect in the control. A configuration check cannot answer a question about
bytes, and I do not think it is meant to.
What I have, and what I am asking. I have been building the other half: for every
revision that lands, replay the merge from the approved parents, compute which shipped
bytes no approval covers, and issue that as a signed in-toto attestation, so a consumer
recomputes the verdict from a clone without trusting the forge that merged it or me. It
is measured against 810 production merges with no false positives, and the predicate is
in review as in-toto/attestation#581.
It composes with REVIEW_ENFORCED rather than replacing it: yours establishes the gate
exists, mine establishes what went through it.
Where it honestly stands: four repositories run it and all four are mine, so it has no
outside adoption. It has had one outside contributor, who found a real divergence in my
multi-parent replay — Git's octopus strategy does no rename detection and merge-tree
does, so a clean octopus merge with a rename/rename conflict replayed as a false
residual. That is fixed and fixtured.
So, two questions:
- Is revision-level evidence for
[Final revision approved] in scope for source-tool,
or deliberately out of it? If it is in scope I would rather contribute to it here than
maintain a parallel thing.
- If anyone runs it on a repository I have never seen and gets a residual they believe
is wrong, that is the report I most want. My own history cannot show me the cases I
did not think of.
Code: https://github.com/DrVelvetFog/source-review-coverage
A pass establishes review coverage and nothing else: not correctness, not that a human
read anything, not truthful authorship, not reviewer independence, not existence at a
time, not compliance.
REQUIREMENTS_MAPPING.md maps Two Party Review to
REVIEW_ENFORCED, which checks that arepository requires pull requests, an approval, dismissal of stale approvals, code-owner
review, and approval of the most recent reviewable push.
Those are properties of the repository's configuration at the moment the control runs.
The requirement they map to is a property of a revision:
The two come apart in at least two places, and I would rather ask whether you consider
them in scope than assume either way.
A merge whose result nobody saw. "Require branches to be up to date before merging"
is a separate setting and is commonly off, because turning it on serialises a busy repo.
With it off, a pull request approved against base A merges into base B, and the tree that
lands is one no reviewer was shown. Every
REVIEW_ENFORCEDcheck still passes: there wasa PR, there was an approval, no commit was pushed after it. The approval was real and it
covered a different tree.
The control is evaluated now; the revision was merged then. A repository that
relaxed a ruleset for an afternoon, or set these rules up last month, reports the same
REVIEW_ENFORCEDtoday as one that always had them. The configuration is current state;the history is not covered by it.
Neither is a defect in the control. A configuration check cannot answer a question about
bytes, and I do not think it is meant to.
What I have, and what I am asking. I have been building the other half: for every
revision that lands, replay the merge from the approved parents, compute which shipped
bytes no approval covers, and issue that as a signed in-toto attestation, so a consumer
recomputes the verdict from a clone without trusting the forge that merged it or me. It
is measured against 810 production merges with no false positives, and the predicate is
in review as in-toto/attestation#581.
It composes with
REVIEW_ENFORCEDrather than replacing it: yours establishes the gateexists, mine establishes what went through it.
Where it honestly stands: four repositories run it and all four are mine, so it has no
outside adoption. It has had one outside contributor, who found a real divergence in my
multi-parent replay — Git's octopus strategy does no rename detection and
merge-treedoes, so a clean octopus merge with a rename/rename conflict replayed as a false
residual. That is fixed and fixtured.
So, two questions:
[Final revision approved]in scope forsource-tool,or deliberately out of it? If it is in scope I would rather contribute to it here than
maintain a parallel thing.
is wrong, that is the report I most want. My own history cannot show me the cases I
did not think of.
Code: https://github.com/DrVelvetFog/source-review-coverage
A pass establishes review coverage and nothing else: not correctness, not that a human
read anything, not truthful authorship, not reviewer independence, not existence at a
time, not compliance.