-
Notifications
You must be signed in to change notification settings - Fork 0
feat!: .NET 10 Support, Dropped .NET 6 #497
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
WalkthroughUpdates project target frameworks to net8.0, net9.0, and net10.0 (removing net6.0), adds a GitHub Actions workflow for NuGet package publishing, removes GitVersion build configuration, and adds unit tests for Ensure context conversions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #497 +/- ##
===========================================
+ Coverage 99.77% 100.00% +0.22%
===========================================
Files 162 57 -105
Lines 437 178 -259
Branches 82 33 -49
===========================================
- Hits 436 178 -258
+ Misses 1 0 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 (1)
tests/NetEvolve.Guard.Tests.Unit/EnsureContextTests.cs (1)
1-160: NewEnsureContexttests give strong coverage forToT()and implicit conversions.The suite hits value types, nullable value types (both null and non-null), reference types (including null and a simple complex object), plus the implicit conversion operator, which is exactly what tends to regress here. The structure is clear and easy to extend; if you ever want to reduce repetition you could consolidate some of the scalar cases into parameterized tests, but that’s purely optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
.github/workflows/publish-nuget.yml(1 hunks)Directory.Build.props(1 hunks)src/NetEvolve.Guard/NetEvolve.Guard.csproj(1 hunks)tests/NetEvolve.Guard.Tests.Unit/EnsureContextTests.cs(1 hunks)tests/NetEvolve.Guard.Tests.Unit/NetEvolve.Guard.Tests.Unit.csproj(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/NetEvolve.Guard.Tests.Unit/EnsureContextTests.cs (1)
src/NetEvolve.Guard/Ensure.cs (1)
Ensure(11-78)
⏰ 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 & Tests / Build Solution / Build .NET solution
🔇 Additional comments (3)
.github/workflows/publish-nuget.yml (1)
1-27: Workflow-based NuGet publish wiring looks solid; please verify artifact/input assumptions.Triggering on
workflow_runof theCIworkflow and gating onconclusion == 'success'with bot exclusions is a good pattern, and the permissions/concurrency setup looks reasonable for a publish workflow.The only things I’d suggest double‑checking are:
- That the CI workflow actually uploads artifacts named
release-packages-*so theartifactPatternmatches.- That
publish-nuget.yml@1.2.27indailydevops/pipelinesstill expects the same input names (workflowName,artifactPattern,environment,runId) and doesn’t require additional permissions (for exampleid-token: write).tests/NetEvolve.Guard.Tests.Unit/NetEvolve.Guard.Tests.Unit.csproj (1)
1-12: Test project TFMs are aligned with the library; change looks good.Targeting
net8.0;net9.0;net10.0here matches the mainNetEvolve.Guardproject’s TFMs and the PR’s goal of droppingnet6.0. Just make sure your CI/build agents have the corresponding SDKs installed so all target frameworks are actually exercised.src/NetEvolve.Guard/NetEvolve.Guard.csproj (1)
3-15: TFM update (drop net6, add net10) matches the intended breaking change.Having
netstandard2.0;net8.0;net9.0;net10.0here is consistent with the tests and with the “feat!: .NET 10 Support, Dropped .NET 6” intent. From a project file perspective this looks good; just ensure your release notes and NuGet package documentation clearly advertise the new supported frameworks and the droppednet6.0target.
Summary by CodeRabbit
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.