Skip to content

Walk the rest of the ruleset, not only the required checks #55

Description

@iderex

The parity document walks the required status checks of the target gate and says
kept, renamed or dropped for each. A required set is only part of what stands
behind a merge. The rest of the ruleset decides who may push, what a merge does
to the commits it lands, and whether anybody has to read a change, and none of
that has been walked against the target.

Print both rather than trusting this body:

gh api repos/Flowfin/lab/rules/branches/main --jq '.[].type'
gh api repos/Flowfin/jellyfin-plugin-sso/rules/branches/main --jq '.[].type'
gh api repos/Flowfin/lab/rules/branches/main \
  --jq '.[] | select(.type=="pull_request") | .parameters'

At the time of writing both boards refuse deletion, refuse a non-fast-forward
push and require a pull request, and the target adds the required status checks
that issue #26 lands here. What is left is the pull-request rule's own
parameters and the rules neither board carries.

The one deviation worth closing rather than reasoning away is the set of merge
methods. The target allows a merge commit only. This board allows a merge
commit, a squash and a rebase, and the last two rewrite the commits they land.
That is not a style preference here. The code-removal decision requires a record
to name the commit that removed the code, and that line is written in the same
change as the removal, so it names a commit that a squash replaces with a
different one. The record then points at nothing on the default branch while
still looking complete. The hand-over path has the same shape one step further
out, because a promotion section names a commit range in this repository.
Restrict the methods to what keeps a named commit resolvable, and write that
reason at the restriction rather than only here.

The parameters that stay as they are, each with its line of reasoning, because a
setting left at its default and a setting chosen deliberately look identical
afterwards. The approving review count is zero on both boards and stays zero
here, since a count above zero on a board where I am the only reviewer refuses
every merge, and a rule nobody can satisfy gets switched off in a hurry rather than
met. Stale-review dismissal, last-push approval and thread resolution are off on
both and stay off, because each of them only bites where a review is required
and none of them means anything at a count of zero. Bypass actors are empty on
both, and #26 already requires that they stay empty.

One rule neither board carries is a verified signature on every commit. This
issue does not add it and does not argue against it. Whether to require one is a
question about key custody rather than an engineering judgement, so it is an
entry on issue #46 and this document points there instead of deciding.

The walk belongs in docs/quality-parity.md as a second section rather than in a
file of its own, so a reader asking what stands behind a merge here finds the
whole answer in one place instead of the checks in one document and the ruleset
in another.

Done when docs/quality-parity.md carries a section listing every rule type the
commands above print for both boards, every parameter of the pull-request rule
with kept or changed and a reason for each change, the merge methods on this
board are restricted to the set that section names with the reason written at the
restriction, and the section names issue #46 for the one decision it does not
take.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions