Enable NuGet.org publishing#74
Conversation
Publish RC builds on main merges and stable releases on version tags. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
More reviews will be available in 4 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR establishes end-to-end NuGet packaging and publishing infrastructure for the Harmony library. It introduces shared MSBuild metadata configuration, adds descriptive metadata to three project packages, activates CI/CD automation for NuGet.org publishing with separate handling of regular and symbol packages, and adds a NuGet version badge to the README. ChangesNuGet Package Setup and Publishing
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Comment |
rmunn
left a comment
rmunn
left a comment
There was a problem hiding this comment.
LGTM. I was concerned about the removal of --include-symbols until I saw that <IncludeSymbols>true</IncludeSymbols> had been added to Directory.Build.targets. So that should be fine, and if we discover that the --include-symbols parameter is necessary after all we can just put it back.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/nuget-ci-cd.yml (1)
18-19:⚠️ Potential issue | 🟠 MajorPin GitHub Actions to immutable SHAs. The workflow uses floating major tags:
actions/checkout@v4(lines 18-19)actions/setup-dotnet@v4(lines 34-35)actions/upload-artifact@v4(lines 45-46)Pin each
uses:entry to a full commit SHA to prevent retargeting without a repo diff.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/nuget-ci-cd.yml around lines 18 - 19, The workflow uses floating major tags for GitHub Actions (actions/checkout@v4, actions/setup-dotnet@v4, actions/upload-artifact@v4); replace each `uses:` entry with the corresponding action pinned to its full commit SHA (e.g., actions/checkout@<full-sha>, actions/setup-dotnet@<full-sha>, actions/upload-artifact@<full-sha>) so the CI cannot be retargeted; find the three `uses:` lines in the YAML and update them to the immutable SHAs for each action.
🧹 Nitpick comments (1)
.github/workflows/nuget-ci-cd.yml (1)
37-43: ⚡ Quick winPack the exact Release build that was tested.
dotnet test --configuration Releasevalidates Release binaries, butdotnet packhere can rebuild with a different configuration and publish artifacts that this job never exercised. Make the pack step explicit and reuse the existing build instead of relying on SDK-specific defaults.Suggested change
- dotnet pack /p:PackageVersion="$PACKAGE_VERSION" /p:AssemblyVersion="$ASSEMBLY_VERSION" /p:FileVersion="$FILE_VERSION" + dotnet pack --configuration Release --no-build \ + /p:PackageVersion="$PACKAGE_VERSION" \ + /p:AssemblyVersion="$ASSEMBLY_VERSION" \ + /p:FileVersion="$FILE_VERSION"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/nuget-ci-cd.yml around lines 37 - 43, The Pack step currently calls dotnet pack which can rebuild with defaults; change it to pack the already-tested Release build by either (1) running dotnet build --configuration Release earlier in the "Build & test" (or replace the test step with dotnet build + dotnet test) and then invoke dotnet pack with --no-build and --configuration Release, or (2) modify the existing Pack step to call dotnet pack --no-build --configuration Release and keep the same /p:PackageVersion, /p:AssemblyVersion, /p:FileVersion properties so the artifacts exactly match the Release binaries exercised by dotnet test; update the step named "Build & test" and the step named "Pack" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/nuget-ci-cd.yml:
- Around line 55-72: The publish step currently uses "shopt -s nullglob" so
empty globs make the for-loops no-ops and a release can succeed without any
artifacts; check for zero matches before the loops (or count matched files) for
the regular package glob "src/artifacts/package/release/*.nupkg" (excluding
"*.symbols.nupkg" / "*.snupkg") and for the symbol glob
"src/artifacts/package/release/*.snupkg", and if either expected set is empty
print a clear error and exit non‑zero (exit 1) to fail the job; preserve the
existing push logic that uses "dotnet nuget push" and the "$NUGET_API_KEY"
variable but gate execution on the pre-checks so missing artifacts cause the
pipeline to fail.
---
Outside diff comments:
In @.github/workflows/nuget-ci-cd.yml:
- Around line 18-19: The workflow uses floating major tags for GitHub Actions
(actions/checkout@v4, actions/setup-dotnet@v4, actions/upload-artifact@v4);
replace each `uses:` entry with the corresponding action pinned to its full
commit SHA (e.g., actions/checkout@<full-sha>, actions/setup-dotnet@<full-sha>,
actions/upload-artifact@<full-sha>) so the CI cannot be retargeted; find the
three `uses:` lines in the YAML and update them to the immutable SHAs for each
action.
---
Nitpick comments:
In @.github/workflows/nuget-ci-cd.yml:
- Around line 37-43: The Pack step currently calls dotnet pack which can rebuild
with defaults; change it to pack the already-tested Release build by either (1)
running dotnet build --configuration Release earlier in the "Build & test" (or
replace the test step with dotnet build + dotnet test) and then invoke dotnet
pack with --no-build and --configuration Release, or (2) modify the existing
Pack step to call dotnet pack --no-build --configuration Release and keep the
same /p:PackageVersion, /p:AssemblyVersion, /p:FileVersion properties so the
artifacts exactly match the Release binaries exercised by dotnet test; update
the step named "Build & test" and the step named "Pack" accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b12b3c8a-4f45-4a6f-bdac-aab5338ed2ff
📒 Files selected for processing (7)
.github/workflows/nuget-ci-cd.ymlREADME.mdharmony.slnsrc/Directory.Build.targetssrc/SIL.Harmony.Core/SIL.Harmony.Core.csprojsrc/SIL.Harmony.Linq2db/SIL.Harmony.Linq2db.csprojsrc/SIL.Harmony/SIL.Harmony.csproj
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Should allow us to get away from submodules and use package references instead.
Publish RC builds on main merges and stable releases on version tags.
Summary by CodeRabbit
Chores
Documentation