-
Notifications
You must be signed in to change notification settings - Fork 232
[BED-6524] Build workflow should run on all PRs against main branch #178
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
base: 2.X
Are you sure you want to change the base?
Conversation
WalkthroughWorkflow updated to run on pull_request events for branch 2.X (opened, synchronize), removing push/development triggers. Upgraded GitHub Actions versions (checkout v4, setup-dotnet v4, action-gh-release v2). Retained existing steps (restore, build, zip) with indentation adjustments and no semantic step changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
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: 2
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
29-31: .NET 5 is EOL; add caching and confirm intended SDK.
- Keep if required by the codebase, but it’s EOL and occasionally flaky to provision on runners. If feasible, plan a move to 6.0 LTS.
- Enable NuGet caching to speed restores.
- - name: Setup .NET - uses: actions/setup-dotnet@v4 - with: - dotnet-version: 5.0.x + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + dotnet-version: '5.0.x' + cache: true + cache-dependency-path: | + **/*.sln + **/*.csprojIf you want to evaluate bumping SDKs, I can scan the repo’s TargetFrameworks and suggest a migration path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build (Debug)
🔇 Additional comments (1)
.github/workflows/build.yml (1)
26-26: Checkout v4 upgrade looks good.No concerns; defaults are fine for PR builds.
| branches: [ 2.X ] | ||
| types: [ opened, synchronize ] |
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.
🛠️ Refactor suggestion
🧩 Analysis chain
PR trigger is too narrow; release steps become dead after removing push trigger.
- Only
openedandsynchronizeare handled;reopenedandready_for_revieware common PR events you likely want. - Removing
pushmeans the “Zip” and “Update Rolling Release” steps will never execute (they’re explicitly skipped on PRs), effectively disabling rolling releases in this workflow.
Consider re-introducing push (for 2.X) and broadening PR event types:
on:
- pull_request:
- branches: [ 2.X ]
- types: [ opened, synchronize ]
+ push:
+ branches: [ 2.X ]
+ pull_request:
+ branches: [ 2.X ]
+ types: [ opened, synchronize, reopened, ready_for_review ]If the intent is to fully decouple releases, split the release steps into a separate workflow triggered on push and remove them from this PR-only workflow.
Run this script to verify whether any other workflow currently handles push-based releases; if none, you’re disabling that path:
🏁 Script executed:
#!/bin/bash
echo "Workflows:"
fd -t f .github/workflows
echo -e "\nWorkflows with 'push' triggers (context shown):"
rg -nC3 -g '.github/workflows/*' $'(?m)^on:\\s*\\n\\s*push:'
echo -e "\nOccurrences of action-gh-release across workflows:"
rg -nC2 -g '.github/workflows/*' 'action-gh-release|Rolling Release'Length of output: 826
🏁 Script executed:
#!/bin/bash
echo "Workflow files in .github/workflows:"
find .github/workflows -type f
echo -e "\nPush triggers in workflows (lines starting with ‘push:’ under on:):"
grep -R -n -E '^[[:space:]]*push:' .github/workflows
echo -e "\nRelease action occurrences ('action-gh-release' or 'Rolling Release'):"
grep -R -n -E 'action-gh-release|Rolling Release' .github/workflows || trueLength of output: 598
🏁 Script executed:
#!/bin/bash
echo "publish.yml triggers:"
grep -n -P '^on:' -A5 .github/workflows/publish.yml
echo -e "\nbuild.yml triggers:"
grep -n -P '^on:' -A5 .github/workflows/build.ymlLength of output: 197
Reintroduce push trigger and extend PR events in .github/workflows/build.yml
build.yml only runs on pull_request (opened, synchronize), so the Zip and Rolling Release steps (softprops/action-gh-release) never fire. Update to:
on:
+ push:
+ branches: [2.X]
pull_request:
branches: [2.X]
- types: [opened, synchronize]
+ types: [opened, synchronize, reopened, ready_for_review]Alternatively, move release steps into a separate push-only workflow.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| branches: [ 2.X ] | |
| types: [ opened, synchronize ] | |
| on: | |
| push: | |
| branches: [2.X] | |
| pull_request: | |
| branches: [2.X] | |
| types: [opened, synchronize, reopened, ready_for_review] |
🤖 Prompt for AI Agents
.github/workflows/build.yml lines 5-6: the workflow currently only triggers on
pull_request (opened, synchronize) so push-based release steps never run; update
the workflow to also include a push trigger for the 2.X branch and expand
pull_request types (e.g., opened, synchronize, reopened, ready_for_review) so PR
state changes re-run the workflow, or alternatively move the release steps into
a separate workflow that triggers only on push to 2.X.
| - name: Update Rolling Release | ||
| if: "! startsWith(github.event_name, 'pull_request')" | ||
| uses: softprops/action-gh-release@v1 | ||
| uses: softprops/action-gh-release@v2 |
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.
Release step won’t run; add permissions if re-enabled on push.
Given the current PR-only trigger, this step never runs. If you re-enable it on push, explicitly grant token permissions or it may fail with 403.
jobs:
build:
+ permissions:
+ contents: writeOptionally, make the condition clearer:
- if: "! startsWith(github.event_name, 'pull_request')"
+ if: github.event_name != 'pull_request'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: softprops/action-gh-release@v2 | |
| jobs: | |
| build: | |
| permissions: | |
| contents: write | |
| steps: | |
| - uses: softprops/action-gh-release@v2 | |
| if: github.event_name != 'pull_request' |
🤖 Prompt for AI Agents
.github/workflows/build.yml around line 45: the release step uses
softprops/action-gh-release but is currently on a PR-only trigger so it never
runs; if you re-enable it for push events, explicitly add GITHUB_TOKEN
permissions (e.g., permissions: contents: write; packages: write; actions:
write) to the workflow or to that job so the action won’t fail with 403, and
optionally make the step's condition/if expression clearer (e.g., only run on
tags or main branch) to avoid accidental runs.
Description
Build pipeline is not configured to run on PRs targeting our main branch '2.X'
Also took the opportunity to update github actions versions
Motivation and Context
BED-6524
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by CodeRabbit