Centralize NuGet package versions with Directory.Packages.props#37
Conversation
Agent-Logs-Url: https://github.com/mpaulosky/MyBlog/sessions/ce2b5a2d-06b0-4119-ab23-d7c2aec4846c Co-authored-by: mpaulosky <60372079+mpaulosky@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Centralizes NuGet package version management across the solution by introducing central package version definitions and removing per-project version declarations to prevent version drift.
Changes:
- Added root
Directory.Packages.propsdefining all package versions used acrosssrc/andtests/. - Enabled central package management (
ManagePackageVersionsCentrally) at the repo level. - Removed
Version="..."attributes fromPackageReferenceentries in multiple projects.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit.Tests/Unit.Tests.csproj | Removes per-reference versions so packages resolve from central versions |
| tests/Integration.Tests/Integration.Tests.csproj | Same migration to central versions for integration test dependencies |
| tests/Architecture.Tests/Architecture.Tests.csproj | Same migration to central versions for architecture test dependencies |
| src/Web/Web.csproj | Switches web project package refs to centrally-managed versions |
| src/ServiceDefaults/ServiceDefaults.csproj | Switches service defaults package refs to centrally-managed versions |
| src/AppHost/AppHost.csproj | Switches Aspire AppHost package refs to centrally-managed versions |
| Directory.Packages.props | Adds central package version declarations |
| Directory.Build.props | Enables MSBuild central package management globally |
| <PackageReference Include="bunit" /> | ||
| <PackageReference Include="coverlet.collector" /> | ||
| <PackageReference Include="coverlet.msbuild"> | ||
| <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | ||
| <PrivateAssets>all</PrivateAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="FluentAssertions" Version="8.9.0" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="18.4.0" /> | ||
| <PackageReference Include="NSubstitute" Version="5.3.0" /> | ||
| <PackageReference Include="xunit" Version="2.9.3" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="3.1.5" /> | ||
| <PackageReference Include="FluentAssertions" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" /> | ||
| <PackageReference Include="NSubstitute" /> | ||
| <PackageReference Include="xunit" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" /> |
There was a problem hiding this comment.
coverlet.collector and xunit.runner.visualstudio are typically intended to be test-only dependencies and should be marked as private assets to prevent them from flowing transitively if the project is ever referenced/packed. Consider adding PrivateAssets="all" (and, for coverlet.collector, IncludeAssets="runtime; build; native; contentfiles; analyzers; buildtransitive"), similar to what you already do for coverlet.msbuild.
| <PackageReference Include="Aspire.Hosting.Testing" /> | ||
| <PackageReference Include="coverlet.collector" /> | ||
| <PackageReference Include="FluentAssertions" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" /> | ||
| <PackageReference Include="NSubstitute" /> | ||
| <PackageReference Include="Testcontainers.MongoDb" /> | ||
| <PackageReference Include="xunit" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" /> |
There was a problem hiding this comment.
Same as Unit.Tests: coverlet.collector and xunit.runner.visualstudio should generally be private assets. Adding PrivateAssets="all" (and appropriate IncludeAssets for coverlet) avoids accidental propagation and keeps dependency graphs cleaner.
| <PackageReference Include="coverlet.collector" /> | ||
| <PackageReference Include="FluentAssertions" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" /> | ||
| <PackageReference Include="NetArchTest.Rules" /> | ||
| <PackageReference Include="xunit" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" /> |
There was a problem hiding this comment.
coverlet.collector and xunit.runner.visualstudio should generally be marked with PrivateAssets="all" (and coverlet IncludeAssets), consistent with typical test-project dependency hygiene and to prevent unintended transitive exposure.
| <Nullable>enable</Nullable> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <TreatWarningsAsErrors>true</TreatWarningsAsErrors> | ||
| <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally> |
There was a problem hiding this comment.
Consider moving (or duplicating) ManagePackageVersionsCentrally into Directory.Packages.props as well, so the “central package management” switch lives alongside the central version declarations. This can reduce confusion for future maintainers and makes the central package file more self-describing when viewed in isolation.
| <PackageVersion Include="Aspire.Hosting.MongoDB" Version="13.2.2" /> | ||
| <PackageVersion Include="Aspire.Hosting.Redis" Version="13.2.2" /> | ||
| <PackageVersion Include="Aspire.Hosting.Testing" Version="13.2.2" /> | ||
| <PackageVersion Include="Aspire.MongoDB.Driver" Version="13.2.2" /> | ||
| <PackageVersion Include="Aspire.StackExchange.Redis.DistributedCaching" Version="13.2.2" /> | ||
| <PackageVersion Include="Auth0.AspNetCore.Authentication" Version="1.7.0" /> | ||
| <PackageVersion Include="Auth0.ManagementApi" Version="8.1.0" /> | ||
| <PackageVersion Include="bunit" Version="2.7.2" /> | ||
| <PackageVersion Include="coverlet.collector" Version="10.0.0" /> | ||
| <PackageVersion Include="coverlet.msbuild" Version="10.0.0" /> | ||
| <PackageVersion Include="FluentAssertions" Version="8.9.0" /> | ||
| <PackageVersion Include="MediatR" Version="14.1.0" /> | ||
| <PackageVersion Include="Microsoft.Bcl.AsyncInterfaces" Version="9.0.6" /> | ||
| <PackageVersion Include="Microsoft.Extensions.Http.Resilience" Version="10.5.0" /> | ||
| <PackageVersion Include="Microsoft.Extensions.ServiceDiscovery" Version="10.5.0" /> | ||
| <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="18.4.0" /> | ||
| <PackageVersion Include="MongoDB.EntityFrameworkCore" Version="10.0.1" /> | ||
| <PackageVersion Include="NetArchTest.Rules" Version="1.3.2" /> | ||
| <PackageVersion Include="NSubstitute" Version="5.3.0" /> | ||
| <PackageVersion Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.15.2" /> | ||
| <PackageVersion Include="OpenTelemetry.Extensions.Hosting" Version="1.15.2" /> | ||
| <PackageVersion Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.15.1" /> | ||
| <PackageVersion Include="OpenTelemetry.Instrumentation.Http" Version="1.15.0" /> | ||
| <PackageVersion Include="OpenTelemetry.Instrumentation.Runtime" Version="1.15.0" /> | ||
| <PackageVersion Include="Testcontainers.MongoDb" Version="4.11.0" /> | ||
| <PackageVersion Include="xunit" Version="2.9.3" /> | ||
| <PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" /> |
There was a problem hiding this comment.
With this file becoming the single source of truth, keeping PackageVersion entries consistently ordered (e.g., alphabetically by Include) will make future updates and diffs easier to review and reduces the chance of duplicate/missed entries as the list grows.
Closes #32 Add missing shared build properties per PRD requirements: - LangVersion=latest — always use the latest C# language version - EnableNETAnalyzers=true — enable .NET platform analyzers - AnalysisMode=All — enable all analyzer rules - EnforceCodeStyleInBuild=true — enforce .editorconfig code style at build time - CodeAnalysisTreatWarningsAsErrors=false — allow analyzer diagnostics as warnings without failing CI (TreatWarningsAsErrors remains true for compiler errors) ManagePackageVersionsCentrally=true was already added by PR #37 and lives in Directory.Build.props; not duplicated here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Closes #32 Working as Sam (Backend Developer) Adds the remaining PRD-required shared build properties to `Directory.Build.props`. ## Changes | Property | Value | Reason | |----------|-------|--------| | `LangVersion` | `latest` | Always use newest C# features | | `EnableNETAnalyzers` | `true` | Enable .NET platform analyzers | | `AnalysisMode` | `All` | Full analyzer rule coverage | | `EnforceCodeStyleInBuild` | `true` | Enforce `.editorconfig` style at build time | | `CodeAnalysisTreatWarningsAsErrors` | `false` | Analyzer diagnostics remain warnings; compiler errors still fail via `TreatWarningsAsErrors=true` | `ManagePackageVersionsCentrally=true` (PR #37) is **not** duplicated. ## Acceptance Criteria - [x] `Directory.Build.props` contains all PRD-required shared build properties - [x] `CodeAnalysisTreatWarningsAsErrors=false` prevents analyzer rules from breaking CI - [x] No individual `.csproj` overrides conflict with the new properties - [ ] `dotnet build MyBlog.slnx --configuration Release` passes (CI validates) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Centralized NuGet version management across
src/andtests/to eliminate per-project version drift and align with the Phase 2 package governance requirement.Directory.Packages.propswith all unique package versions currently used by solution projects.ManagePackageVersionsCentrally.Versionattributes from projectPackageReferenceentries so they resolve from the central file.nugetupdate target remains at root (directory: "/"), matching centralized package management.Type of Change
Domain Affected
Self-Review Checklist
Code Quality
dotnet build MyBlog.slnx --configuration Release— 0 errors, 0 warningsdotnet test MyBlog.slnx --configuration Release --no-build— all passArchitecture
Command/Query/Handler/Validatornaming conventionssealedWeborPersistence.*projectsResult<T>/ResultErrorCodeused for expected failures (no exception-driven control flow)Domain.DTOs; Models are inDomain.ModelsTests
[Collection("XxxIntegration")])IssueDto.Empty/CommentDto.Emptyinstances directlySecurity (check if security-relevant)
RequireAuthorization/ policy appliedMarkupStringused with user-supplied contentMerge Readiness
main(no merge conflicts)Screenshots / Evidence
Notes for Reviewers
Please focus review on:
Directory.Packages.propspackage/version coverage vs. allsrc/+tests/project references.