Uh oh!
There was an error while loading. Please reload this page.
feat: add parameters for branch protection rule customization - #23
Conversation
📝 WalkthroughWalkthroughRepository settings now use configurable Make variables and a downloaded ChangesBranch protection configuration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MakeRepoSettings
participant RepoSettingsScript
participant GitHub
participant WorkflowRepository
MakeRepoSettings->>RepoSettingsScript: Pass REPO_* variables and tool settings
RepoSettingsScript->>GitHub: Authenticate and reconcile repository settings
RepoSettingsScript->>GitHub: Update or create protect-main ruleset
RepoSettingsScript->>WorkflowRepository: Download update-action-pins workflow
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@common.mk`:
- Line 117: Update the generated ruleset summary echo in the relevant make
target to print the generated branch targets from RULESET_JSON’s
conditions.ref_name.include field instead of relying only on
REPO_RULESET_BRANCHES, so the default ~DEFAULT_BRANCH is shown even when the
variable is empty.
- Around line 121-127: The required status-check input handling in common.mk
must preserve spaces within individual context names instead of splitting
REPO_STATUS_CHECKS on whitespace; switch to an array-valued input or equivalent
lossless encoding and update the corresponding README.md description to document
the new format.
- Around line 144-148: Update common.mk around the ruleset generation to reject
REPO_REQUIRE_BRANCH_UP_TO_DATE=true when REPO_STATUS_CHECKS is empty, failing
before generating a ruleset without required_status_checks; alternatively,
update README.md line 114 to explicitly state that branch freshness requires at
least one REPO_STATUS_CHECKS value. Apply the chosen behavior consistently at
common.mk lines 144-148 and README.md line 114.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 88ad4943-7d3a-46d8-8247-028b9be2b4f4
📒 Files selected for processing (3)
README.mdcommon.mkdocs/NEW_REPO.md
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
424af78 to
a0f4c21Compareolzemal
commented
Aug 6, 2026
@coderabbitai review |
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
a0f4c21 to
7277d0dCompareThere was a problem hiding this comment.
Two things before merge:
$(RULESET_JSON)vs$$RULESET_JSONslip in the twogh apicalls (inline below). It ships an empty body to the API, so the target doesn't actually create or update the ruleset- The recipe is now ~40 lines of shell + jq inlined in the Makefile, every line escaped with
\and$$. Theshellchecktarget only lintsgit ls-files '*.sh', so none of this shell is covered. Wdyt about moving the body intohack/repo-settings.shand passing theREPO_*values as env? shellcheck picks it up for free, the jq program can live in ajq -ffile without the escaping.. Doesn't need to be this PR, i can sketch it if useful.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
7277d0d to
d6fe687CompareNote GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
d6fe687 to
ede5afaCompareThere was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/repo-settings.sh`:
- Around line 34-35: Normalize REPO_STATUS_CHECKS once to a list containing only
non-empty string contexts, then use that normalized value for validation,
required-status-check ruleset creation, and reporting. In the
REPO_REQUIRE_BRANCH_UP_TO_DATE guard, reject an empty normalized list, and
update the logic around the required-status-checks installation and output to
reuse the same effective list so logs cannot describe unapplied checks.
- Around line 29-32: Update the repository selection logic in
scripts/repo-settings.sh to preserve and use an explicitly configured REPO
value, only resolving the current GitHub repository when REPO is unset. Also
update common.mk to forward REPO when invoking this script, ensuring
REPO=owner/name targets the requested repository.
- Around line 139-141: Update the workflow download in scripts/repo-settings.sh
to first write to a temporary file within .github/workflows, then atomically
move that file to .github/workflows/update-action-pins.yml only after curl
succeeds. Ensure failed or interrupted downloads do not replace or truncate the
existing workflow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a9670ec6-588b-4539-919e-ef017e49883f
📒 Files selected for processing (4)
README.mdcommon.mkdocs/NEW_REPO.mdscripts/repo-settings.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- README.md
- common.mk
- docs/NEW_REPO.md
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ede5afa to
3dbedb8Compareolzemal
commented
Aug 6, 2026
@coderabbitai review |
✅ Action performedReview finished.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
cbrgm
commented
Aug 7, 2026
That looks good now. I haven't tested it against a fake repo yet, but I would suppose you did right? Otherwise looks good to me :) |
olzemal
commented
Aug 7, 2026
Yes, probably should have mentioned it in the description... I tested it against a random fork repo of mine :D |
What
related to https://github.com/opendefensecloud/odd-internal/issues/44
common.mk- the staticREPO_RULESETjson is replaced with a ruleset built at runtime via jq, driven by new make variablesmake repo-settingsnow prints the effective ruleset settings on each run.README.md/docs/NEW_REPO.md- documented the new variables.Checklist
Summary by CodeRabbit
New Features
Documentation