-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update CI workflows and project files for improved package management and versioning #632
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
Conversation
📝 WalkthroughWalkthroughConsolidates CI/release workflows: upgrades actions to v5, centralizes packaging to a single dotnet pack producing Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant GH as GitHub (release)
participant WF as Workflow
participant SDK as dotnet SDK
participant Art as Release Assets
rect rgb(230,245,255)
Dev->>GH: Create/publish release (tag)
GH-->>WF: Trigger release.published
WF->>WF: verify refs/tags/*\nderive version from tag
end
rect rgb(245,255,230)
WF->>SDK: dotnet pack --output out/ /p:PackageVersion=<derived>
WF->>Art: Upload files in out/* as release assets (GitHub API)
end
rect rgb(255,245,230)
opt optional NuGet push
WF->>SDK: dotnet nuget push out/*.nupkg --source configured --api-key NUGET_AUTH_TOKEN
end
end
sequenceDiagram
autonumber
participant Repo as Repo (push/PR)
participant CI as CI Workflow
participant SDK as dotnet SDK
participant Art as Artifacts
Repo-->>CI: Trigger CI workflow
CI->>SDK: dotnet pack -> out/
CI->>Art: Upload out/* artifacts
opt configured publish
CI->>SDK: Push packages to NuGet/source (token)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
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: 1
🧹 Nitpick comments (2)
Directory.Build.props (1)
14-14: Consider the prerelease suffix strategy.The
-previewsuffix inVersionPrefixmeans all builds (including release builds) will be marked as prerelease by default. The release workflow overrides this with-p:Version=$version, which works, but it creates an inconsistency where:
- CI builds:
1.6.1-preview.{BUILD_NUMBER}- Release builds: Version from tag (e.g.,
1.6.1)If this is intentional to mark non-release builds as preview, this is fine. However, if release builds should use
1.6.1without-preview, consider:
- Using
<VersionPrefix>1.6.1</VersionPrefix>(without-preview)- Having CI workflow add prerelease suffix:
-p:VersionSuffix=preview.$GITHUB_RUN_NUMBERThis would make the versioning strategy clearer and ensure release builds use clean version numbers by default.
.github/workflows/wf-build-release.yml (1)
47-48: Misleading step name.The step name says "Publish packages to Github packages" but it's actually publishing to NuGet.org (configured in the setup-dotnet step at line 34). Consider renaming to "Publish packages to NuGet.org" for clarity.
- - name: Publish packages to Github packages + - name: Publish packages to NuGet.org run: dotnet nuget push "out/*" -k ${{secrets.NUGET_AUTH_TOKEN}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/wf-build-release-ci.yml(2 hunks).github/workflows/wf-build-release.yml(1 hunks)Directory.Build.props(1 hunks)QRCoder.Xaml/QRCoder.Xaml.csproj(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/wf-build-release.yml
51-51: the runner of "actions/github-script@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: additional-tests
🔇 Additional comments (11)
.github/workflows/wf-build-release-ci.yml (5)
18-18: LGTM! Actions upgraded to v5.The checkout and setup-dotnet actions have been upgraded from v4 to v5, which is the latest version and includes security improvements and new features.
Also applies to: 22-22, 80-80, 84-84
87-89: LGTM! NuGet source configured for GitHub Packages.The NuGet source configuration correctly points to the GitHub Packages repository and uses the GITHUB_TOKEN for authentication, which is the standard approach for publishing to GitHub Packages.
91-92: Consolidated packing step improves workflow efficiency.The workflow now consolidates build and pack into a single step, which simplifies the CI process. The version will be formatted as
1.6.1-preview.{BUILD_NUMBER}by combining:
VersionPrefixfrom Directory.Build.props:1.6.1-previewVersionSuffixfrom this command:$GITHUB_RUN_NUMBERThis results in versions like
1.6.1-preview.1234for CI builds, clearly distinguishing them from release builds.
94-98: LGTM! Artifact upload preserves build outputs.The artifact upload step correctly captures all NuGet packages from the output directory, making them available for download and inspection.
100-101: LGTM! Simplified NuGet push command.The push command correctly uses a wildcard pattern to publish all packages and authenticates with the GITHUB_TOKEN, working in conjunction with the NuGet source configured in the setup-dotnet step.
.github/workflows/wf-build-release.yml (5)
1-6: LGTM! Release-driven workflow trigger.The workflow now triggers automatically on release publication, which streamlines the release process and ensures consistency between GitHub releases and NuGet package publication.
15-20: LGTM! Defensive validation for tag references.The validation step ensures the workflow only processes tag-based releases, preventing accidental execution on non-tag refs. The error message clearly indicates the failure reason.
30-36: LGTM! NuGet source configured for public package publishing.The .NET SDK setup correctly configures NuGet.org as the source and uses the NUGET_AUTH_TOKEN secret for authentication, which is the standard approach for publishing public NuGet packages.
38-39: LGTM! Version override for clean release versions.The pack command correctly overrides the Version property with the extracted tag version, ensuring release packages have clean version numbers (e.g.,
1.6.1) instead of the preview suffix from VersionPrefix in Directory.Build.props.
22-28: Handle optionalvprefix in version extraction.
Extraction via${github_ref:10}removesrefs/tags/but retains a leadingv(e.g.,v1.2.3). Ensure downstream commands accept this or strip it with:version="${version#v}"QRCoder.Xaml/QRCoder.Xaml.csproj (1)
32-32: README.md presence confirmed
README.md exists in QRCoder.Xaml with appropriate content for packaging.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/wf-build-release.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). (1)
- GitHub Check: build
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/wf-build-release.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). (2)
- GitHub Check: additional-tests
- GitHub Check: build
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/wf-build-release-ci.yml(2 hunks).github/workflows/wf-build-release.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/wf-build-release-ci.yml
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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). (2)
- GitHub Check: additional-tests
- GitHub Check: build
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: 0
🧹 Nitpick comments (1)
Directory.Build.props (1)
13-14: Consider separating version prefix and suffix for better flexibility.The change to
VersionPrefixis appropriate, but the conventional MSBuild pattern separates the numeric version from the pre-release identifier:<VersionPrefix>1.6.1</VersionPrefix> <VersionSuffix>preview</VersionSuffix>This approach provides more flexibility for CI/CD workflows to manipulate version components independently (e.g., switching between
preview,beta, or removing the suffix for release builds).However, if the CI workflows are designed to override this value from repository tags, the current approach may work fine.
Please verify that the CI/CD workflows correctly handle the version format and can override or manipulate it as needed for different build scenarios (development, preview, release).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Directory.Build.props(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: additional-tests
- GitHub Check: build
- GitHub Check: format
Summary by CodeRabbit
Chores
Documentation
Versioning