chore: add CODEOWNERS for docs, ADRs, constants, and tooling - #650
Open
Rafiat30 wants to merge 1 commit into
Open
chore: add CODEOWNERS for docs, ADRs, constants, and tooling#650Rafiat30 wants to merge 1 commit into
Rafiat30 wants to merge 1 commit into
Conversation
Changes to shared constants, ADRs, and deployment/CI scripts had no review gate: anyone could merge a change to bettapay_common's constants, an accepted ADR, or the scripts CI itself invokes without a second set of eyes on it, letting cross-cutting drift slip in silently. - Add .github/CODEOWNERS mapping adr/, bettapay_common/src/constants.rs, scripts/, Makefile, .github/workflows/, and the top-level contributor docs (CONTRIBUTING.md, SECURITY.md, DEVELOPMENT.md) to a reviewer. Owner is @therealjhay, this repository's most active contributor by a wide margin. - Add scripts/check_codeowners.sh, which validates the file itself: every pattern must still resolve to a real path in the repository (catches a gated path being renamed or removed without updating CODEOWNERS), and every owner must be a syntactically valid @user or @org/team handle. Wired into `make all` as a new check_codeowners target. - Add scripts/tests/check_codeowners_smoke_test.sh, covering a valid file, a pattern with no owner, a malformed owner, a pattern that doesn't resolve, and a missing CODEOWNERS file entirely - wired into the existing test_scripts target alongside tooling_smoke_test.sh. - Add a "Code Ownership" section to CONTRIBUTING.md documenting which paths are gated and why, and explaining the two-part enforcement: this new CI check keeps the file itself correct, while making review from code owners mandatory (rather than advisory) requires enabling "Require review from Code Owners" under branch protection for `main` - a repo setting outside this file's reach. Closes Betta-Pay#547
|
@Rafiat30 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #547
Summary
Changes touching shared constants, ADRs, or the deployment/CI scripts had no owner gate: a PR could merge a change to
bettapay_common's constants, an accepted ADR, or the scripts CI itself invokes with no second set of eyes on it, letting cross-cutting drift slip in silently.This adds
.github/CODEOWNERS, a script + smoke test that validate the file stays correct as the repo evolves, and aCONTRIBUTING.mdsection documenting the review process (including the one piece — GitHub's own "Require review from Code Owners" branch protection setting — that has to be turned on outside the repo).Changes
New files
.github/CODEOWNERS— mapsadr/,bettapay_common/src/constants.rs,scripts/,Makefile,.github/workflows/, and the top-level contributor docs (CONTRIBUTING.md,SECURITY.md,DEVELOPMENT.md) to@therealjhay, this repository's most active contributor by a wide margin (237 contributions vs. the next-highest at 21, per the GitHub contributors API). Each block has a comment explaining why that path is gated, not just what's gated.scripts/check_codeowners.sh— validates.github/CODEOWNERSitself: every pattern must still resolve to a real file or directory in the working tree (so a gated path silently losing its reviewer requirement when renamed/removed gets caught), and every owner must be a syntactically valid@useror@org/teamhandle. Follows the existingscripts/check_wasm_size.shpattern (sourcesscripts/lib/common.sh, honors aROOT_DIRoverride for testability).scripts/tests/check_codeowners_smoke_test.sh— exercises the validation script against fixture CODEOWNERS files in amktemp -dsandbox, following the same pattern as the existingscripts/tests/tooling_smoke_test.sh.Modified files
CONTRIBUTING.md— new "Code Ownership" section (linked from the table of contents) listing the gated paths, why each is gated, and how enforcement works.Makefile— added acheck_codeownerstarget wired intomake all, and added the new smoke test to the existingtest_scriptstarget.No contract (Rust) source changed in this PR.
Implementation details
gh api repos/Betta-Pay/BettaPay-Contract/contributorsand picked the real top contributor. This can be changed by editing one file once the project has a formal maintainer team.check_codeowners.shdoes, is keep the CODEOWNERS file itself honest: no unowned pattern, no malformed owner, no pattern pointing at a path that no longer exists. Both halves are documented in the new CONTRIBUTING.md section so it's clear which part is automated here and which part needs a one-time admin action.check_codeowners.shintentionally does simple substring/regex validation rather than trying to fully implement GitHub's gitignore-style glob matching — every pattern actually in use in this file is a plain path, so a direct[ -e ... ]existence check is sufficient and keeps the script simple to audit.Tests added
scripts/tests/check_codeowners_smoke_test.shcovers, each against a disposable fixture repo inmktemp -d:@) fails.I also manually verified
scripts/check_codeowners.shagainst the real.github/CODEOWNERSadded in this PR (passes) and against an intentionally-broken copy (correctly reports both a bad owner and an unresolvable pattern with file:line detail) before finalizing the smoke test.How to test