Skip to content

Conversation

@definitelynotagoblin
Copy link
Contributor

@definitelynotagoblin definitelynotagoblin commented Sep 24, 2025

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

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Documentation updates are needed, and have been made accordingly.
  • I have added and/or updated tests to cover my changes.
  • All new and existing tests passed.
  • My changes include a database migration.

Summary by CodeRabbit

  • Chores
    • Updated CI to run on pull requests targeting 2.X only.
    • Refreshed GitHub Actions versions for checkout, .NET setup, and release steps.
    • Minor workflow formatting adjustments.
    • No user-facing changes.

@definitelynotagoblin definitelynotagoblin self-assigned this Sep 24, 2025
@definitelynotagoblin definitelynotagoblin added the bug Something isn't working label Sep 24, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 24, 2025

Walkthrough

Workflow 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

Cohort / File(s) Summary of Changes
CI Workflow Updates
\.github/workflows/build.yml
Narrowed triggers to pull_request on 2.X (opened, synchronize); upgraded actions to latest majors (actions/checkout@v4, actions/setup-dotnet@v4, softprops/action-gh-release@v2); minor indentation alignment; no functional changes to restore/build/zip steps.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • MikeX777

Poem

I twitch my ears at branch 2.X tonight,
New actions hop to v4—what a sight! 🐇
No pushes now, just PRs in flight,
Restore, build, zip—steady and light,
A tidy warren, workflows right.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description follows the repository template structure and includes a Description, Motivation and Context, Types of Changes, and a Checklist, but the “How Has This Been Tested?” section remains empty, leaving out critical details on how the workflow updates were validated. Without specifics on the test environment, commands run, or results observed, reviewers cannot confirm that the changes function as intended. Please fill out the “How Has This Been Tested?” section with detailed information about the testing environment, commands or scripts executed, and the outcomes observed to demonstrate that the updated workflow triggers and GitHub Actions versions work correctly.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly summarizes the primary change by stating that the build workflow will run on all pull requests against the main branch and includes the relevant issue identifier. It is concise, focused on the core update, and avoids unnecessary details or ambiguity, making it easy for reviewers and teammates to understand the purpose at a glance. The phrasing is clear, uses a single sentence, and accurately reflects the main change in the pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch anemeth/BED-6524

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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
+            **/*.csproj

If 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

📥 Commits

Reviewing files that changed from the base of the PR and between d53cc04 and 6223d72.

📒 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.

Comment on lines +5 to +6
branches: [ 2.X ]
types: [ opened, synchronize ]
Copy link

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 opened and synchronize are handled; reopened and ready_for_review are common PR events you likely want.
  • Removing push means 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 || true

Length 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.yml

Length 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.

Suggested change
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
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

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: write

Optionally, 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.

Suggested change
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants