-
Notifications
You must be signed in to change notification settings - Fork 1
github-modify: Allow specifying default branch #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughIntroduces a new optional input Changes
Sequence Diagram(s)sequenceDiagram
actor W as Caller Workflow
participant A as github-modify (composite action)
participant G as actions/checkout
W->>A: Run with inputs (default_branch?)
alt Alternate branch provided elsewhere
A->>G: Checkout ref: <override-branch>
note right of G: Uses explicit override
else No override provided
A->>G: Checkout ref: inputs.default_branch (default: master)
note right of G: Uses configurable default branch
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
8b91eb7
to
5dd9b89
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
github-modify/action.yml (1)
10-14
: Remove non-standard input property "type" from action metadataIn action.yml for composite actions, inputs don’t support a "type" field (that’s for reusable workflows via workflow_call). Keeping it is harmless in many cases but not spec-compliant and could confuse maintainers. Recommend dropping it here. The default will still make the value a string.
Apply this diff:
default_branch: description: "The default branch to check out if no overrides" required: false - type: string default: 'master'
Note: The "repo" input above also has
type: string
(unchanged in this PR). For consistency and spec compliance, consider removing that in a separate, follow-up change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
github-modify/action.yml
(2 hunks)
🔇 Additional comments (1)
github-modify/action.yml (1)
58-58
: LGTM: final checkout now honors configurable default branchSwitching the fallback ref from a hard-coded 'master' to
${{ inputs.default_branch }}
aligns with the PR goal and keeps existing behavior via the provided default. No functional concerns.If you want to sanity-check behavior across repos whose default is "main", trigger this action with
with: default_branch: main
on a test workflow and confirm the checkout SHA matches the repository’s default branch tip.
5dd9b89
to
d888ba7
Compare
Signed-off-by: mulhern <amulhern@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
github-modify/action.yml (1)
9-12
: Input looks good; consider clarifying usage and future-proofing for ‘main’.The new optional input is correct and backwards-compatible. To reduce confusion for repos whose default branch is ‘main’, tweak the description to make the default explicit and point users to override it.
Apply this small doc-only diff:
- default_branch: - description: "The default branch to check out if no overrides" - required: false - default: 'master' + default_branch: + description: "Branch to check out in stratis-storage when no fork branch is found (default: 'master'). Set to 'main' for repos that use 'main'." + required: false + default: 'master'If you’d like, I can also add README usage examples showing how a caller sets
default_branch: main
.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
github-modify/action.yml
(2 hunks)
🔇 Additional comments (1)
github-modify/action.yml (1)
56-56
: No remaining hard-coded ‘master’ refs foundI ran the ripgrep search across the repo (excluding vendor dirs) and the only match was the
default: 'master'
on line 12 ofgithub-modify/action.yml
—which is the intended default for the newdefault_branch
input. No other hard-codedmaster
refs remain.✅ All clear—resolving this comment.
Related stratis-storage/project#810
Summary by CodeRabbit