Enable package lock file and update SDK to 9.0.304 #39
Conversation
WalkthroughThe changes update .NET SDK version management to use Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant global.json
participant NuGet
participant MSBuild
GitHub Actions->>global.json: Read SDK version
GitHub Actions->>NuGet: Restore packages using lock file
NuGet->>NuGet: Use cache if lock file unchanged
GitHub Actions->>MSBuild: Build with CI properties set
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
snovak7
changed the title
|
@rabbitcodeai review |
![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
🔭 Outside diff range comments (1)
.github/workflows/build.yml (1)
41-42: Fix brittle if conditions for release steps (avoid referencing github.event.release on non-release events).Current expressions can evaluate unexpectedly and access undefined fields on non-release events. Prefer explicit checks.
Suggested diffs per step:
- if: ${{ github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name }} + if: ${{ github.event_name == 'release' && github.event.action == 'published' }} - if: ${{ github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name }} + if: ${{ github.event_name == 'release' && github.event.action == 'published' }} - if: ${{ github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name }} + if: ${{ github.event_name == 'release' && github.event.action == 'published' }} - if: ${{ github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name }} + if: ${{ github.event_name == 'release' && github.event.action == 'published' }} - if: ${{ env.apikey != '' && github.event_name == 'release' && github.ref_type == 'tag' || github.event.release.tag_name }} + if: ${{ env.apikey != '' && github.event_name == 'release' && github.event.action == 'published' }}Alternative: gate the entire publish block with a single job-level condition or use
contains(github.ref, 'refs/tags/')when building from tags.Also applies to: 67-68, 71-72, 81-82, 88-89
🧹 Nitpick comments (3)
.github/workflows/build.yml (2)
50-51: Optional: make locked-restore intent explicit in CI.CI already sets
RestoreLockedModevia MSBuild props. Adding the CLI flag improves clarity and fails fast if locks are missing.- run: dotnet restore + run: dotnet restore --locked-mode
33-35: Optional: include Directory.Packages.props in cache key if you use central package management.If versions change centrally, cache invalidation becomes more predictable.
- cache-dependency-path: '**/packages.lock.json' + cache-dependency-path: | + **/packages.lock.json + **/Directory.Packages.propsDirectory.Build.props (1)
2-6: Optional: consider adding Deterministic and ContinuousIntegrationBuild at the solution level.Many repos set these explicitly for reproducible builds outside CI as well.
<PropertyGroup> + <Deterministic>true</Deterministic> + <ContinuousIntegrationBuild Condition="'$(CI)' == 'true'">true</ContinuousIntegrationBuild> <RestoreLockedMode Condition="'$(CI)' == 'true'">true</RestoreLockedMode> <RestorePackagesWithLockFile>true</RestorePackagesWithLockFile> </PropertyGroup>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build.yml(1 hunks)Directory.Build.props(1 hunks)global.json(1 hunks)src/Escendit.Tools.CodeAnalysis.StyleCopAnalyzers/packages.lock.json(1 hunks)
🔇 Additional comments (6)
src/Escendit.Tools.CodeAnalysis.StyleCopAnalyzers/packages.lock.json (2)
1-47: Lock file addition looks good and supports deterministic restores.Targets netstandard2.0/2.1, consistent direct deps, and NuGet lockfile v2 metadata present. Works with
RestoreLockedModein CI.
1-47: No missing packages.lock.json files detectedAll project directories include a committed packages.lock.json, so CI’s RestoreLockedMode won’t fail due to a missing or outdated lock file.
.github/workflows/build.yml (1)
33-35: Good switch to global.json and enabling NuGet cache keyed by lock files.
global-json-file: global.jsonkeeps CI/local aligned;cache: true+cache-dependency-path: '**/packages.lock.json'correctly keys the NuGet cache on lockfile changes.Directory.Build.props (3)
4-6: NuGet lock usage and CI locked mode are configured correctly.
RestorePackagesWithLockFileon andRestoreLockedModeunder CI enforce deterministic restores.
11-12: CI build property is appropriate for SourceLink/PDB determinism.
ContinuousIntegrationBuildgated on CI is a good default.
15-15: Directory.Packages.props found at repo root; central management enabled
TheDirectory.Packages.propsfile is present in the repository root, so<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>will take effect as intended. No further action required.
Closes #38
Summary by CodeRabbit