Skip to content

Conversation

@Shane32
Copy link
Owner

@Shane32 Shane32 commented Oct 2, 2025

Summary by CodeRabbit

  • Chores

    • Reorganized CI into multi-job pipeline (test, coverage upload, additional validations, packaging) with PR trigger, concurrency and cancel-in-progress to reduce duplicate runs.
    • Centralized testing and coverage reporting, removed redundant per-target steps, added trim analysis and API tests, and improved artifact/checkout handling.
  • Build

    • Assembly signing settings now apply only to Release builds, simplifying local/debug builds without API changes.

@Shane32 Shane32 self-assigned this Oct 2, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

📝 Walkthrough

Walkthrough

Restructures CI into a consolidated multi-job pipeline (test, upload-coverage, additional-tests, pack-push-ci) with concurrency and PR triggers; centralizes .NET SDK installs and Codecov uploads; and scopes assembly signing properties to Release builds in two project files. (34 words)

Changes

Cohort / File(s) Summary
CI: consolidated multi-job pipeline
.github/workflows/wf-build-release-ci.yml, .../.github/workflows/wf-build-test.yml
Replaces/reshapes legacy build into multi-job workflow: adds test, upload-coverage, additional-tests, and pack-push-ci jobs; introduces concurrency groups and cancel-in-progress; moves to PR triggers for test workflow; centralizes .NET SDK installation; collects and uploads coverage to Codecov; adds trim analysis and API tests; adjusts packaging/publishing steps and NuGet source/credentials.
Project files: signing gated to Release
QRCoder/QRCoder.csproj, QRCoder.Xaml/QRCoder.Xaml.csproj
Moves SignAssembly, AssemblyOriginatorKeyFile, and DelaySign into a PropertyGroup conditioned on $(Configuration)=='Release', restricting strong-name signing to Release builds only.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Push / PR
  participant GH as GitHub Actions
  participant Test as job:test
  participant Cov as job:upload-coverage
  participant Add as job:additional-tests
  participant Pack as job:pack-push-ci

  Dev->>GH: Push or PR triggers workflow
  GH->>Test: Checkout → Install .NET SDK → Run tests → Produce coverage artifact
  Test-->>Cov: Publish/download coverage artifact
  Cov->>Cov: Prepare coverage files → Upload to Codecov
  GH->>Add: Checkout → Install .NET SDK → Run trim analysis & API tests
  par Dependencies complete
    Test-->>GH: test result
    Add-->>GH: additional-tests result
    Cov-->>GH: coverage upload result
  end
  GH->>Pack: On dependencies success → Checkout → Install .NET SDK → Pack → Publish packages
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • gfoidl

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately captures the main scope of the pull request, which is to update the CI workflows for pushing to the master branch, and it is clear and specific enough for team members to understand the primary change at a glance.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-ci-workflow

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a01700 and 7656c0e.

📒 Files selected for processing (1)
  • .github/workflows/wf-build-release-ci.yml (2 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

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.

@Shane32 Shane32 changed the title Update CI workflow for pushing to master [WIP] Update CI workflow for pushing to master Oct 2, 2025
Copy link
Contributor

@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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90df7dd and 9cfd5fa.

📒 Files selected for processing (3)
  • .github/workflows/wf-build-release-ci.yml (1 hunks)
  • QRCoder.Xaml/QRCoder.Xaml.csproj (1 hunks)
  • QRCoder/QRCoder.csproj (1 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). (3)
  • GitHub Check: build
  • GitHub Check: test
  • GitHub Check: additional-tests

Copy link
Contributor

@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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cfd5fa and d027061.

📒 Files selected for processing (1)
  • .github/workflows/wf-build-release-ci.yml (1 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). (3)
  • GitHub Check: build
  • GitHub Check: additional-tests
  • GitHub Check: test

Copy link
Contributor

@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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/wf-build-test.yml (1)

15-16: Remove deprecated ACTIONS_ALLOW_UNSECURE_COMMANDS

This flag is deprecated and unnecessary here; keeping it weakens security posture.

-    env:
-        ACTIONS_ALLOW_UNSECURE_COMMANDS: true
🧹 Nitpick comments (2)
.github/workflows/wf-build-test.yml (1)

32-36: Avoid uploading the entire workspace; add checkout to test job and only ship build outputs

Uploading $GITHUB_WORKSPACE is slow and costly. Since the test job doesn’t checkout the repo, it needs source files. Add checkout to test and restrict artifacts to bin outputs.

     - name: Upload artifacts
       uses: actions/upload-artifact@v4
       with:
         name: Compiled project
-        path: ${{ github.workspace }}
+        path: |
+          QRCoder/**/bin/**
+          QRCoder.Xaml/**/bin/**
+          QRCoderTests/**/bin/**

Also add checkout to the test job:

   test:
     needs: build
     runs-on: windows-latest
     strategy:
       fail-fast: false
     name: Test ${{ matrix.dotnet-framework.name }}
     steps:
+    - uses: actions/checkout@v4
+      name: Checkout Code
     - name: Download artifacts
       uses: actions/download-artifact@v4

Also applies to: 38-41, 54-59

.github/workflows/wf-build-release-ci.yml (1)

40-42: Make coverage matching explicit and pass to Codecov

Tighten the glob to the actual coverlet output and pass it to Codecov to avoid discovery misses.

-        path: QRCoderTests/coverage.*.xml
+        path: QRCoderTests/coverage*.opencover.xml
     - name: Upload to Codecov
       uses: codecov/codecov-action@v4
       with:
         token: ${{ secrets.CODECOV_TOKEN }}
         fail_ci_if_error: true
+        files: coverage*.opencover.xml

Also applies to: 53-58

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d027061 and 2b2947e.

📒 Files selected for processing (3)
  • .github/workflows/wf-build-release-ci.yml (1 hunks)
  • .github/workflows/wf-build-test.yml (2 hunks)
  • QRCoder/QRCoder.csproj (1 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). (3)
  • GitHub Check: build
  • GitHub Check: additional-tests
  • GitHub Check: test
🔇 Additional comments (5)
.github/workflows/wf-build-test.yml (2)

8-11: Good use of PR-scoped concurrency

Prevents overlapping PR runs; cancel-in-progress is appropriate.


42-42: Disabling fail-fast is appropriate for matrix tests

Ensures all target frameworks report results.

QRCoder/QRCoder.csproj (1)

60-64: Signing gated to Release: LGTM

Limits strong-name signing to Release builds; improves local dev ergonomics.

Confirm QRCoder.Xaml mirrors this change (AI summary says yes) and that any consumers expecting signed Debug assemblies are updated.

.github/workflows/wf-build-release-ci.yml (2)

10-13: Concurrency per ref/workflow: LGTM

Prevents overlapping CI runs on the same ref.


26-31: Verify availability of EOL SDKs (2.1.x/3.1.x)

windows-latest images may no longer host or allow installing these. Keep only if strictly required; otherwise drop to reduce flakiness.

@Shane32 Shane32 changed the title [WIP] Update CI workflow for pushing to master Update CI workflow for pushing to master Oct 2, 2025
@Shane32 Shane32 requested a review from gfoidl October 2, 2025 14:11
@Shane32
Copy link
Owner Author

Shane32 commented Oct 2, 2025

@gfoidl I'm proactively merging this, but lmk if you have any comments and I can address them in a follow-up PR.

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.

2 participants