Skip to content

docs: add branch-protection checklist for required CI checks on main#32

Merged
naimkatiman merged 1 commit intomainfrom
docs/branch-protection-checklist
Apr 27, 2026
Merged

docs: add branch-protection checklist for required CI checks on main#32
naimkatiman merged 1 commit intomainfrom
docs/branch-protection-checklist

Conversation

@naimkatiman
Copy link
Copy Markdown
Owner

Summary

  • Adds docs/branch-protection-checklist.md as a one-shot paste-and-click reference for lifting this session's three CI lints from advisory to required on the main branch
  • Single file, +111 LOC, documentation-only

Why now

This session shipped three CI lints across PRs #28, #30, and #31 that all enforce different correctness rules — but none are currently required on main. The merge button is enabled regardless of whether the lints pass. The checklist closes that gap with a one-time GitHub-UI configuration that takes ~2 minutes.

The doc covers:

  • UI path (Settings → Branches → Add rule for main) with the exact boxes to tick
  • CLI alternative (gh api PUT with a JSON body, since branch-protection nested fields are awkward via --field flags)
  • Verification step — open any PR and confirm the four checks show Required badges
  • Revert path — fully reversible from the same UI or via gh api -X DELETE
  • Why now — names the three lints (skill-mirror, docs-substrings, 7-laws verifier) that become load-bearing once test is required, and references the session demonstration where PR feat(ci): add skill-mirror lint enforcing CONTRIBUTING.md mirror rule #28 had a red test (20) and the merge button still offered to merge

Required checks proposed

Required check Job What it enforces
test (18) test matrix on Node 18 Build + tests + 7-laws verifier + instinct-pack JSON + skill-mirror parity + docs-substring assertions
test (20) test matrix on Node 20 Same, on Node 20
test (22) test matrix on Node 22 Same, on Node 22
lint-transcript lint-transcript job Verifies bin/lint-transcript.mjs runs without errors

The skill-mirror, docs-substrings, 7-laws, and instinct-pack-JSON lints all live as steps inside the test job, so making test (N) required for each Node version covers all four with a single setting.

What this PR does NOT do

  • It does not apply the branch-protection rule itself. Branch-protection settings are repo-wide configuration that survives in the GitHub repo metadata, not in the codebase. The doc tells you the exact clicks; you make the change in the GitHub UI (or via gh api if you prefer).
  • The doc commits only the plan; the application is your call at your own pace.

Test plan

  • Read the doc end-to-end and confirm the click path matches your current GitHub UI (settings paths occasionally rename)
  • Verify each of the four required-check names matches the actual check names that appear on a recent PR (gh pr checks <PR> lists them with the names GitHub uses internally)
  • Optionally: apply the rule to main, then open a fresh PR (or push a commit to an existing branch) and confirm all four checks show Required badges before merging

PR scope discipline

Following all rules in CONTRIBUTING.md:

  • One concern. Title is "add branch-protection checklist" — no "and", no comma.
  • Size budget. 1 file, +111/-0 LOC.
  • No drive-bys. No other doc edits, no changes to existing files.
  • Bundle regen rule. N/A — pure docs.
  • Skill mirror rule. N/A — no skill files touched.
  • Test/docs sync rule. N/A — no assert.match on docs/branch-protection-checklist.md exists.

Out of scope (intentionally)

  • Applying the branch-protection rule itself — that's a one-click action you do after reading the checklist
  • Adding a CI check that verifies the branch-protection settings are correct (recursive enforcement) — overkill for a single-maintainer repo

🤖 Generated with Claude Code

Adds docs/branch-protection-checklist.md as a one-shot paste-and-click
reference for lifting the CI lints from advisory to required on the
main branch.

Covers the UI path (Settings → Branches), the gh api equivalent for
CLI users, the verification step (look for "Required" badges on an
open PR), and the reverting path. Names the four required checks
that need to be added (test 18/20/22, lint-transcript) and explains
that the skill-mirror, docs-substrings, and 7-laws lints all run
inside the test job so making test required covers all three with a
single setting.

Closes the natural end of the rule-then-lint discipline arc shipped
across PRs #27, #28, #30, #31 — three lints landed but none required
until this UI change is applied.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new documentation file, docs/branch-protection-checklist.md, which provides a detailed guide for setting up branch protection on the main branch using both the GitHub UI and the CLI. The feedback points out two critical issues in the instructions: the GitHub UI does not allow setting the required number of approvals to zero when pull requests are mandatory, and the API payload in the CLI example will fail because the approval count must be between 1 and 6. A suggestion was provided to set the review requirement to null in the API call to resolve this.

Comment on lines +26 to +27
- [ ] **Require a pull request before merging**
- Sub-option: **Require approvals: 0** (single-maintainer repo; raise later if you take on collaborators)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In the GitHub UI, if Require a pull request before merging is enabled, the Required number of approvals before merging must be at least 1. It cannot be set to 0. For a single-maintainer repository, it is generally recommended to leave Require a pull request before merging unchecked and only enable Require status checks to pass before merging. This allows you to merge your own PRs once CI passes without needing a second account for approval.

Comment on lines +56 to +60
"required_pull_request_reviews": {
"required_approving_review_count": 0,
"dismiss_stale_reviews": false,
"require_code_owner_reviews": false
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The GitHub API will return a 422 Unprocessable Entity error if required_approving_review_count is set to 0, as the valid range is 1-6. To disable the requirement for approvals while still using the protection endpoint, set required_pull_request_reviews to null.

Suggested change
"required_pull_request_reviews": {
"required_approving_review_count": 0,
"dismiss_stale_reviews": false,
"require_code_owner_reviews": false
},
"required_pull_request_reviews": null,

@naimkatiman naimkatiman merged commit c519b04 into main Apr 27, 2026
4 checks passed
@naimkatiman naimkatiman deleted the docs/branch-protection-checklist branch April 27, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant