mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-28 07:37:49 +09:00
Myasnikov Daniil
16a49740fe
CODEOWNERS applies only the last matching pattern and it replaces every broader rule including the catch-all, it does not merge them. Half of what that costs is already fixed:bd7b76691added the architects to the rules that were missing them, and hack/codeowners-invariant.bats keeps them there. That invariant reads "every rule with owners is a superset of the catch-all owners", and a rule naming nothing but the architects satisfies it perfectly while gating on two people. Eleven rules on main have that shape, 182 files between them. The catch-all is those same two handles, so the invariant makes them the floor for the whole repository and every path without a rule of its own sits on that floor. Raising the floor means raising the catch-all, after which the invariant carries the new set into every rule by itself. Hence the shape here: the catch-all goes from two owners to five, and all fifty rules carry those five. This widens who may approve, not who may change code. Write access is unchanged for all twelve collaborators, owner approval is OR and always was, and the required checks that actually hold these paths are untouched, api-review-gate.yaml included, so a sizeable API change still needs an approval from one of its own two owners. Three of the nine maintainers in MAINTAINERS.md own no path, for two different reasons.c13c6ee2cgave kingdonb the two FluxCD tenant addon packages and7cca3d75edeleted them along with the addon, so his rules went with the packages while the flux paths that stayed were never handed to him. matthieu-robin and mattia-eleuteri were never in the file at all. CONTRIBUTOR_LADDER.md says a maintainer may approve changes to any area of the project. Rules now group by area rather than by package, every rule names at least five owners, and an area whose owner set does not differ from its tier is left to the tier rule. Reviewers keep the areas MAINTAINERS.md records for them, so that file needs no change. The two-owner rule on /cmd/api-gate/, /internal/apigate/ and the api-review-gate workflow goes with the rest. Those paths still require an owner review, now under the Go source and CI rules. The invariant test's exemption for a rule owned by exactly tym83 and kvaps is dead code once the governance block is gone, and dead code in that position is a hole: any future two-owner rule matching that set would be waived. Dropping it puts such a rule back under the invariant, where it belongs. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-by: LLM
43 lines
1.8 KiB
Bash
43 lines
1.8 KiB
Bash
#!/usr/bin/env bats
|
|
# -----------------------------------------------------------------------------
|
|
# Coverage invariant for .github/CODEOWNERS.
|
|
#
|
|
# CODEOWNERS applies only the LAST matching rule, and matching is not
|
|
# additive: a scoped rule fully replaces the * catch-all instead of extending
|
|
# it. The only way to keep the default owners able to review everything is to
|
|
# repeat their handles in every scoped rule. A rule that omits one of them
|
|
# silently carves its path out from under the catch-all, and since "Require
|
|
# review from Code Owners" is enforced on main and stale reviews are dismissed
|
|
# on push, that path then deadlocks whenever its remaining owners are away,
|
|
# with no way around it short of an admin override.
|
|
#
|
|
# The invariant: every rule that lists owners must include every owner of the
|
|
# * catch-all. Ownerless rules are the one exception, being generated files
|
|
# whose review is waived on purpose.
|
|
#
|
|
# Changing the catch-all owners is a policy change; this test failing on such
|
|
# a change is the point - adjust the expectation here in the same PR, with the
|
|
# policy discussion linked.
|
|
# -----------------------------------------------------------------------------
|
|
|
|
@test "every CODEOWNERS rule that lists owners repeats the catch-all owners" {
|
|
[ -f .github/CODEOWNERS ]
|
|
|
|
awk '
|
|
/^[[:space:]]*(#|$)/ { next }
|
|
$1 == "*" { for (i = 2; i <= NF; i++) defaults[$i]; ndef = NF - 1; next }
|
|
NF < 2 { next }
|
|
{
|
|
for (d in defaults) {
|
|
found = 0
|
|
for (i = 2; i <= NF; i++) if ($i == d) found = 1
|
|
if (!found) { printf "line %d: rule %s is missing default owner %s\n", FNR, $1, d; bad = 1 }
|
|
}
|
|
}
|
|
END {
|
|
if (ndef == 0) { print "no * catch-all rule with owners found"; exit 1 }
|
|
exit bad
|
|
}
|
|
' .github/CODEOWNERS
|
|
}
|