Remove NuGet secret in condition - #48
Merged
snovak7 merged 3 commits intoApr 30, 2026
Merged
Conversation
…via input flags, remove NUGET_ORG_KEY requirement
Reviewer's guide (collapsed on small PRs)Reviewer's Guide.github workflow tweaks to optimize test execution and adjust NuGet artifact upload/publish conditions by decoupling them from a secret-based guard and tying upload to the pack input. Flow diagram for updated .NET test step in CI workflowflowchart TD
A[Job build] --> B[Step .NET Test]
B --> C{inputs.test is true?}
C -- No --> D[Skip test step]
C -- Yes --> E[Run dotnet test]
E --> F[Use --no-restore]
E --> G[Use --no-build]
E --> H[Use --nologo]
E --> I[Collect XPlat Code Coverage]
E --> J[Use test-adapter-path .]
E --> K[Configure Coverlet output]
E --> L[Configure Coverlet format]
E --> M[Configure Coverlet exclude-by-file]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
ifexpressions combining&&and||(e.g. for the upload artifact and NuGet.org publish jobs) would be clearer and less error-prone with explicit parentheses around the OR clause, e.g.inputs.pack && (github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name)→inputs.pack && (github.event_name == 'release' && (github.ref_type == 'tag' || github.event.release.tag_name))(or similar, depending on the intended logic). - By removing
&& secrets.NUGET_ORG_KEYfrom the publish job condition, the workflow will now start that job even when the secret is missing; consider adding an early dedicated step that checks for the secret and fails with a clear message to make this failure mode more explicit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `if` expressions combining `&&` and `||` (e.g. for the upload artifact and NuGet.org publish jobs) would be clearer and less error-prone with explicit parentheses around the OR clause, e.g. `inputs.pack && (github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name)` → `inputs.pack && (github.event_name == 'release' && (github.ref_type == 'tag' || github.event.release.tag_name))` (or similar, depending on the intended logic).
- By removing `&& secrets.NUGET_ORG_KEY` from the publish job condition, the workflow will now start that job even when the secret is missing; consider adding an early dedicated step that checks for the secret and fails with a clear message to make this failure mode more explicit.
## Individual Comments
### Comment 1
<location path=".github/workflows/dotnet-package.yml" line_range="116-121" />
<code_context>
- name: .NET Upload NuGet Artifact
uses: actions/upload-artifact@v7
- if: ${{ github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name }}
+ if: ${{ inputs.pack && (github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name) }}
with:
name: nuget
</code_context>
<issue_to_address>
**suggestion:** Clarify boolean precedence in the `if` expression with extra parentheses for maintainability.
This currently relies on GitHub Actions’ `&&`/`||` precedence, which is harder to read and easy to mis-edit. Please group the release condition explicitly, e.g.: `if: ${{ inputs.pack && ((github.event_name == 'release' && github.ref_type == 'tag') || github.event.release.tag_name) }}`.
```suggestion
- name: .NET Upload NuGet Artifact
uses: actions/upload-artifact@v7
if: ${{ inputs.pack && ((github.event_name == 'release' && github.ref_type == 'tag') || github.event.release.tag_name) }}
with:
name: nuget
path: packages/
name: NuGet.org Publish
runs-on: ubuntu-latest
needs: build
if: ${{ inputs.nuget-publish && ((github.event_name == 'release' && github.ref_type == 'tag') || github.event.release.tag_name) }}
steps:
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/dotnet-package.yml" line_range="201-203" />
<code_context>
name: NuGet.org Publish
runs-on: ubuntu-latest
needs: build
- if: ${{ inputs.nuget-publish && (github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name) && secrets.NUGET_ORG_KEY }}
+ if: ${{ inputs.nuget-publish && (github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name) }}
steps:
# GitHub Download NuGet Artifact [for NuGet.org Repository]
</code_context>
<issue_to_address>
**issue (bug_risk):** Dropping the `secrets.NUGET_ORG_KEY` guard may cause the publish job to run without a configured API key.
Previously this job was skipped when `NUGET_ORG_KEY` was missing, avoiding a failing publish attempt. Now it will run whenever `inputs.nuget-publish` and the release conditions are true, even if the secret isn’t configured. Unless this check has been moved into the publish step, please keep a guard like `&& secrets.NUGET_ORG_KEY` so misconfigurations fail fast and clearly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis pull request modifies the dotnet CI/CD workflow by optimizing test execution with skip flags, adding a conditional gate to artifact uploads based on pack inputs, and removing an invalid Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 0/1 reviews remaining, refill in 46 minutes and 25 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/dotnet-package.yml:
- Line 203: The NuGet publish job is gated only by inputs.nuget-publish and
release info, but the package artifact is uploaded only when inputs.pack is
true, causing missing-artifact failures; update the job's if condition to
require inputs.pack as well (i.e., change the if expression that currently
references inputs.nuget-publish to also && inputs.pack) so that the NuGet
publish step (the conditional using inputs.nuget-publish) runs only when packing
was enabled; ensure the logical grouping/parentheses preserve the existing
release/tag checks while adding inputs.pack to the condition.
- Around line 82-84: The dotnet test invocation uses --no-build while the build
was done in Release, so update the dotnet test command to include the explicit
configuration flag (add --configuration Release alongside --no-build) so test
discovery uses the built Release artifacts; modify the existing dotnet test line
to pass --configuration Release when --no-build is present.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41296269-6f63-411c-8b54-1d014fc93ec2
📒 Files selected for processing (1)
.github/workflows/dotnet-package.yml
snovak7
deleted the
47-reusable-workflow-remove-nuget-key-secret-in-a-condition
branch
April 30, 2026 10:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #47
Summary by Sourcery
Adjust .NET CI workflow conditions and test execution flags for package publishing and test runs.
Enhancements:
Summary by CodeRabbit