Conversation
Co-authored-by: KarateJB <13954352+KarateJB@users.noreply.github.com>
Co-authored-by: KarateJB <13954352+KarateJB@users.noreply.github.com>
KarateJB
force-pushed
the
copilot/upgrade-csharp-to-dotnet-8
branch
from
40bcca8 to
3213502
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the project from .NET Core 2.0 to .NET 8.0, modernizing APIs and dependencies across the solution.
- Updated target frameworks from netcoreapp2.0/netstandard2.0 to net8.0
- Migrated from deprecated ASP.NET Core APIs to modern equivalents
- Updated NuGet package versions to current stable releases
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| CSharp/DP.Website/DP.Website.csproj | Upgraded target framework to net8.0 and removed obsolete package references |
| CSharp/DP.Website/Startup.cs | Updated to use IWebHostEnvironment instead of IHostingEnvironment and AddControllersWithViews instead of AddMvc |
| CSharp/DP.Website/Views/Home/State.cshtml | Replaced deprecated Html.Partial with modern partial tag helper syntax |
| CSharp/DP.Website/Views/Home/Builder.cshtml | Replaced deprecated Html.Partial with modern partial tag helper syntax |
| CSharp/DP.UnitTest/DP.UnitTest.csproj | Upgraded target framework to net8.0 and updated test-related package versions |
| CSharp/DP.UnitTest/Utility/XunitTraceListener.cs | Added exception handling for InvalidOperationException when no active test |
| CSharp/DP.Domain/DP.Domain.csproj | Upgraded target framework to net8.0 and suppressed SYSLIB0011 warning |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _env = env; | ||
| } | ||
|
|
||
| public IConfiguration Configuration { get; } |
There was a problem hiding this comment.
The Configuration property is never assigned and will always be null. The constructor was changed to accept IWebHostEnvironment instead of IConfiguration, but the property wasn't updated accordingly. Either remove this unused property or add configuration injection to the constructor.
| for (int i = 0; i < Model.Parents.Count(); i++) | ||
| { | ||
| @Html.Partial("_ParentPartial", Model.Parents.ToList()[i]) | ||
| <partial name="_ParentPartial" model="Model.Parents.ToList()[i]" /> |
There was a problem hiding this comment.
Calling ToList() inside the loop on every iteration is inefficient. The enumerable is materialized multiple times. Consider calling ToList() once before the loop and storing the result in a variable.
| for (int i = 0; i < Model.Children.Count(); i++) | ||
| { | ||
| @Html.Partial("_ChildPartial", Model.Children.ToList()[i]) | ||
| <partial name="_ChildPartial" model="Model.Children.ToList()[i]" /> |
There was a problem hiding this comment.
Calling ToList() inside the loop on every iteration is inefficient. The enumerable is materialized multiple times. Consider calling ToList() once before the loop and storing the result in a variable.
| for (int i = 0; i < Model.Pets.Count(); i++) | ||
| { | ||
| @Html.Partial("_PetPartial", Model.Pets.ToList()[i]) | ||
| <partial name="_PetPartial" model="Model.Pets.ToList()[i]" /> |
There was a problem hiding this comment.
Calling ToList() inside the loop on every iteration is inefficient. The enumerable is materialized multiple times. Consider calling ToList() once before the loop and storing the result in a variable.
| for (int i = 0; i < Model.Parents.Count(); i++) | ||
| { | ||
| @Html.Partial("_ParentPartial", Model.Parents.ToList()[i]) | ||
| <partial name="_ParentPartial" model="Model.Parents.ToList()[i]" /> |
There was a problem hiding this comment.
Calling ToList() inside the loop on every iteration is inefficient. The enumerable is materialized multiple times. Consider calling ToList() once before the loop and storing the result in a variable.
| for (int i = 0; i < Model.Children.Count(); i++) | ||
| { | ||
| @Html.Partial("_ChildPartial", Model.Children.ToList()[i]) | ||
| <partial name="_ChildPartial" model="Model.Children.ToList()[i]" /> |
There was a problem hiding this comment.
Calling ToList() inside the loop on every iteration is inefficient. The enumerable is materialized multiple times. Consider calling ToList() once before the loop and storing the result in a variable.
| for (int i = 0; i < Model.Pets.Count(); i++) | ||
| { | ||
| @Html.Partial("_PetPartial", Model.Pets.ToList()[i]) | ||
| <partial name="_PetPartial" model="Model.Pets.ToList()[i]" /> |
There was a problem hiding this comment.
Calling ToList() inside the loop on every iteration is inefficient. The enumerable is materialized multiple times. Consider calling ToList() once before the loop and storing the result in a variable.
Summary
Successfully upgraded all C# projects from .NET Core 2.0/netstandard 2.0 to .NET 8.
Changes Made:
Results:
✅ Build succeeds without errors
✅ No vulnerable packages
✅ No CodeQL alerts
✅ XunitTraceListener throws exceptions as designed
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.