Skip to content

Include main branch ruleset as part of the template#40

Open
d-morrison wants to merge 7 commits into
mainfrom
claude/include-branch-ruleset
Open

Include main branch ruleset as part of the template#40
d-morrison wants to merge 7 commits into
mainfrom
claude/include-branch-ruleset

Conversation

@d-morrison

@d-morrison d-morrison commented May 19, 2026

Copy link
Copy Markdown
Owner

Adapted from d-morrison/rpt#134.

Summary

  • Export the live main branch ruleset to .github/rulesets/main.json (server-assigned fields stripped so it round-trips through the create/update endpoints).
  • New .github/scripts/apply-rulesets.sh idempotently applies every JSON in .github/rulesets/ to the current repo — PUT to update an existing ruleset by name, POST to create.
  • New .github/rulesets/README.md documents what's enforced (required PR, required status checks for Spellcheck / check-chars / build-deploy, no force-push / no deletion, Maintain-role PR-only bypass) and how to re-export after editing in the UI.
  • README.Rmd / README.md: add a one-step entry under "Setup steps" pointing to the script.

Test plan

  • Create a throwaway repo from the template and run .github/scripts/apply-rulesets.sh against it — verify the ruleset shows up under Settings → Rules → Rulesets with all required status checks listed.
  • Re-run the script — confirm it updates in place rather than creating a duplicate.
  • Edit a rule in .github/rulesets/main.json, run the script, confirm the change appears in the GitHub UI.

🤖 Generated with Claude Code

d-morrison and others added 3 commits May 18, 2026 20:31
Adapted from d-morrison/rpt#134. Exports the live `main` ruleset to
.github/rulesets/main.json with the server-assigned fields stripped, adds
apply-rulesets.sh that PUTs to update / POSTs to create (idempotent), and
documents what's enforced in .github/rulesets/README.md. README.Rmd /
README.md get a step pointing to the script under "Setup steps".

qbt-specific ruleset: required PR + required status checks (link-checker,
Spellcheck, check-chars, build-deploy; non-strict) + no force-push / no
deletion, Maintain-role PR-only bypass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The check-links.yml workflow declares `pull_request` as a trigger but
doesn't actually produce a check run on PR events in this repo (workflow
run history shows only `schedule` events firing it). Requiring it as a
status check would make every PR wait forever for a context that never
arrives.

The live ruleset on the canonical qbt repo currently has the same gap;
this commit removes it from the JSON shipped with the template so that
new repos derived from the template don't inherit a broken gate. Apply
the change to the live repo by running `.github/scripts/apply-rulesets.sh`.

Also clarify in the rulesets README that the `build-deploy` PR check is
satisfied by `preview.yml` (publish.yml is push-only), so future renames
don't silently break the ruleset gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "Apply branch rulesets" step described the required-checks list as
(link-checker, Spellcheck, check-chars, build-deploy). link-checker was
removed from main.json in the previous commit; update the README to
match so the docs and the actual ruleset don't disagree.

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

Copy link
Copy Markdown
Owner Author

@claude review

@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 1m 32s —— View job


Code Review

  • Gather context
  • Read changed files
  • Review .github/rulesets/main.json
  • Review .github/scripts/apply-rulesets.sh
  • Review .github/rulesets/README.md
  • Review README.Rmd / README.md changes
  • Post review findings

Overall this is well-structured work. The idempotent PUT/POST logic is the right approach for a template-based setup, and the documentation is clear. A few things worth noting:

.github/rulesets/main.json

required_approving_review_count: 0 — A PR is required before merging, but zero approvals are needed, so authors can self-merge. This may be intentional for a small team, but it's worth documenting explicitly in the rulesets README.md so template users understand they're getting process enforcement (PR required) but not peer-review enforcement. Anyone expecting this to block self-merges will be surprised.

strict_required_status_checks_policy: false — Branches don't need to be up to date before merging. This is documented in the README and reasonable for the intended workflow, but it means a stale branch could pass CI and still cause integration failures after merging. Low risk for this use case.

bypass_actors[0].actor_id: 2 — Role ID 2 is the standard GitHub "Maintain" role, which is stable across repos. This is correct.

.github/scripts/apply-rulesets.sh

The script is solid. A couple of minor edge cases:

name extraction could produce the string "null" — If a .json file in the rulesets directory happens to lack a .name field, jq -r .name returns the string "null". The script would then query for an existing ruleset named "null", find nothing, and attempt to POST — likely failing with a GitHub API error. A defensive guard would help: Fix this →

# after: name=$(jq -r .name "$f")
if [ -z "$name" ] || [ "$name" = "null" ]; then
  echo "skipping $f: missing .name field" >&2
  continue
fi

No jq presence checkjq is listed as a requirement in the README, but the script will fail with a cryptic command not found error rather than a clear message. Minor nit given the target audience (admins), but a fast-fail at the top would improve UX: Fix this →

.github/rulesets/README.md

Well-written. The rationale for excluding check-links.yml is particularly helpful context — future maintainers will know not to add it back without rethinking the schedule.

The re-export docs use a hardcoded RULESET_ID placeholder. A small improvement would be to show how to find the ID inline:

# Find the ID:
gh api repos/OWNER/REPO/rulesets | jq '.[] | select(.name == "main") | .id'

README.Rmd / README.md

Clean. The new step 2 is well-placed and describes the operation and its effects clearly. Numbering shift is handled correctly. The "(requires admin access)" callout is accurate and useful.


Summary: Two minor defensive improvements to the shell script are worth considering, and documenting the zero-approvals behavior in README.md would prevent surprise. Nothing blocking.

@d-morrison

Copy link
Copy Markdown
Owner Author

Claude Code CLI (local session) is working on this — paws off until I'm done.

- apply-rulesets.sh: fail fast with a clear message if jq is missing
- apply-rulesets.sh: skip ruleset files lacking a .name field (avoid a
  bogus POST for a ruleset literally named "null")
- rulesets/README.md: document the zero-approvals (self-merge) behavior and
  show how to look up the ruleset ID inline before re-export
- README.Rmd/README.md: note the zero-approvals behavior in the apply step

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@d-morrison

Copy link
Copy Markdown
Owner Author

Addressed all review observations:

apply-rulesets.sh

  • Added a fast-fail command -v jq check at the top with a clear error message.
  • Added a guard that skips any ruleset file lacking a .name field (prevents a bogus POST for a ruleset named "null").

rulesets/README.md

  • Documented the required_approving_review_count: 0 behavior: a PR is required but zero approvals are needed, so authors can self-merge (process enforced, peer review not).
  • Showed how to look up the ruleset ID inline before re-export.

README.Rmd / README.md

  • Noted the zero-approvals / self-merge behavior in the apply-rulesets step (kept both in sync).

strict_required_status_checks_policy: false and bypass_actors were noted as correct/reasonable in the review, so left as-is.

Re-requesting review.

@d-morrison

Copy link
Copy Markdown
Owner Author

@claude review

@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 1m 8s —— View job


Code Review

  • Gather context
  • Read changed files
  • Verify previous findings were addressed
  • Check for new issues
  • Post review findings

All four findings from the previous review are correctly addressed. Here's a brief verification pass:

apply-rulesets.sh

  • jq fast-fail (line 13): ✅ command -v jq >/dev/null 2>&1 || { echo "error: jq is required but not found" >&2; exit 1; } — clean and correct.
  • Null-name guard (lines 31–34): ✅ if [ -z "$name" ] || [ "$name" = "null" ] covers both the empty-string and literal-"null" cases.
  • The idempotent PUT/POST logic still looks correct: // empty on the jq id lookup produces an empty shell variable when no match is found, and the -n "$id" branch test handles it cleanly.

.github/rulesets/README.md

  • Zero-approvals / self-merge: ✅ Clearly documented (lines 26–29) with actionable guidance to raise required_approving_review_count if peer review is wanted.
  • Inline ID lookup: ✅ Added as RULESET_ID=$(gh api repos/OWNER/REPO/rulesets | jq '.[] | select(.name == "main") | .id') before the re-export command. Note: .id is a number, so jq outputs it unquoted — this is correct as-is; no -r flag needed for numeric values used in a URL path.

README.Rmd / README.md

  • ✅ Zero-approvals / self-merge callout is in the setup step in both files and kept in sync. Numbering renumbering is handled correctly. The additional Acknowledgments section and removal of the stale link-checker manual-override note are clean bonus improvements.

main.json

  • No server-assigned fields present; round-trips cleanly through the create/update endpoints.

Nothing blocking. The implementation is solid and the documentation is clear for template users. Ready to merge.

Copy link
Copy Markdown
Owner Author

Claude Code (remote session, ARDIA sweep) is picking this up — paws off until done.


Generated by Claude Code

claude added 2 commits June 20, 2026 09:25
The spellcheck CI was failing because 'rulesets' (added to README.Rmd:110
and README.md:111 in this PR) is not in the dictionary.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SNxbcZHy5tYHWAzeJ9XL4J
Copilot AI review requested due to automatic review settings June 20, 2026 09:25

Copy link
Copy Markdown
Owner Author

ARD summary (ARDIA sweep)

# Finding Disposition Detail
CI: Spellcheck failing "rulesets" not in dictionary (README.md:111, README.Rmd:110) ✅ Address Added "rulesets" to inst/WORDLIST. Also merged current main into the branch. Pushed in eb28861.

Round-2 review verdict was "Ready to merge. No blocking issues." — all code findings were already addressed. This push re-triggers CI; once Spellcheck goes green the PR is fully clean.


Generated by Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds an exportable, re-applicable main branch ruleset to the template so repos created from it can programmatically enforce consistent branch protections (via a gh + jq script), and documents how to apply/maintain those rules.

Changes:

  • Adds .github/rulesets/main.json plus documentation describing what the ruleset enforces and how to re-export it.
  • Adds .github/scripts/apply-rulesets.sh to idempotently create/update all rulesets in .github/rulesets/ via the GitHub API.
  • Updates README.Rmd / README.md setup steps to include applying rulesets; updates the spellcheck wordlist.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
README.Rmd Adds a setup step instructing admins to apply the branch ruleset after creating a repo from the template.
README.md Same setup-step update as README.Rmd in rendered Markdown.
inst/WORDLIST Adds “rulesets” to the spellcheck allowlist.
.github/scripts/apply-rulesets.sh New script that applies all JSON rulesets to a repo using gh api.
.github/rulesets/README.md Documentation for what the ruleset enforces and how to apply/re-export it.
.github/rulesets/main.json Exported ruleset definition intended to protect the default branch via PR + required checks + no force-push/deletion.

Comment thread .github/scripts/apply-rulesets.sh
Comment thread .github/scripts/apply-rulesets.sh Outdated
Comment thread .github/rulesets/README.md Outdated
Comment thread README.Rmd
Comment thread README.md Outdated
Comment thread .github/rulesets/main.json
- add `gh` dependency check alongside existing `jq` check
- add --paginate to `gh api` rulesets listing so repos with >30 rulesets
  don't silently create duplicates
- fix rulesets/README.md: check-links.yml is excluded because it checks
  external URLs (transient failures, not PR-related), not because it
  doesn't run on PRs — it does have a pull_request trigger
- README.Rmd / README.md: use full path .github/rulesets/main.json

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SNxbcZHy5tYHWAzeJ9XL4J

Copy link
Copy Markdown
Owner Author

ARD summary — Copilot round (commit a99c62b)

# Finding Disposition Detail
1 apply-rulesets.sh:13 — no gh dependency check; unclear error if missing Addressed Added command -v gh fast-fail alongside the existing jq check.
2 apply-rulesets.sh:27gh api without --paginate; repos with >30 rulesets silently create duplicates Addressed Changed to `gh api --paginate …
3 README.md:122 — refers to main.json; full path reduces ambiguity Addressed Changed to .github/rulesets/main.json.
4 README.Rmd:119 — same as above Addressed Changed to .github/rulesets/main.json.
5 rulesets/README.md:36 — says check-links.yml "not on PRs"; it does have a pull_request trigger Addressed Replaced incorrect reason ("runs on schedule, not PRs") with the actual reason: it checks external URLs that can fail due to transient network issues or link rot unrelated to the PR, which would block merges on failures outside the author's control.
6 main.json:45 — PR description listed link-checker as required; JSON doesn't include it Addressed Updated PR description to remove link-checker / from the required-checks list. The rulesets README now also accurately explains the exclusion.

Push triggered CI re-run.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://d-morrison.github.io/qbt/pr-preview/pr-40/

Built to branch gh-pages at 2026-06-20 09:37 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

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.

3 participants