chore: update EF Core packages and add dev tooling#8
Conversation
📝 WalkthroughWalkthroughAdds a design-time EF Core DbContext factory, upgrades the project and EF/Npgsql packages to .NET 10, introduces a migration and updated model snapshot adjusting identity column types and discriminator, removes a duplicate DbContext registration in startup, and adds a justfile with development tasks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Program.cs (1)
13-16: Consider usingInvalidOperationExceptioninstead of the baseException.Throwing
System.Exceptionmakes it harder for callers (or global exception handlers) to distinguish configuration errors from other failures. A more specific type likeInvalidOperationExceptionbetter communicates intent and is consistent with the factory class added in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Program.cs` around lines 13 - 16, Replace the generic throw new Exception in Program.cs when connectionString is empty with a more specific InvalidOperationException to clearly signal a configuration error; update the throw site that checks string.IsNullOrEmpty(connectionString) so it throws new InvalidOperationException with the same descriptive message (or include nameof for the environment variable) to match the factory-class style used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CarpentryShop.csproj`:
- Around line 5-6: The XML comment on the csproj is doubly-escaped and
unreadable; replace or remove the garbled comment node so the project file
contains a normal XML comment or none at all. Locate the RuntimeIdentifier
element and the adjacent commented line (the node that contains "<!–
This line is the temp fix –>") and either delete that commented node
or rewrite it as a standard XML comment such as <!-- This line is the temp fix
--> so it renders correctly and no HTML entities remain.
In `@Program.cs`:
- Around line 42-45: The code registers UseHttpsRedirection middleware twice in
non-development mode: once guarded by app.Environment.IsDevelopment() (negated)
and once unconditionally; remove the duplicate by deleting the unconditional
call to app.UseHttpsRedirection() so the middleware is only added when
!app.Environment.IsDevelopment(); locate the unconditional
app.UseHttpsRedirection() invocation (the second occurrence) and remove it,
leaving the existing guarded block (if (!app.Environment.IsDevelopment()) {
app.UseHttpsRedirection(); }) intact.
- Around line 59-65: The startup code is calling Database.EnsureCreated() which
conflicts with EF Core migrations; update the scope block that obtains
CarpentryShopIdentityDbContext via app.Services.CreateScope() to either remove
the EnsureCreated call entirely (so the justfile/db-update handles migrations)
or replace context.Database.EnsureCreated() with context.Database.Migrate() to
apply migrations at startup; ensure you keep the same scope/service resolution
(CreateScope, GetRequiredService<CarpentryShopIdentityDbContext>) and only
change the Database.* call accordingly.
---
Nitpick comments:
In `@Program.cs`:
- Around line 13-16: Replace the generic throw new Exception in Program.cs when
connectionString is empty with a more specific InvalidOperationException to
clearly signal a configuration error; update the throw site that checks
string.IsNullOrEmpty(connectionString) so it throws new
InvalidOperationException with the same descriptive message (or include nameof
for the environment variable) to match the factory-class style used elsewhere.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Areas/Identity/Data/DesignTimeDbContextFactory.csCarpentryShop.csprojMigrations/20260225143925_UpdateModels.Designer.csMigrations/20260225143925_UpdateModels.csMigrations/CarpentryShopIdentityDbContextModelSnapshot.csProgram.csjustfile
| if (!app.Environment.IsDevelopment()) | ||
| { | ||
| app.UseHttpsRedirection(); | ||
| } |
There was a problem hiding this comment.
UseHttpsRedirection is invoked twice in non-development mode.
Lines 42–45 call UseHttpsRedirection() when !IsDevelopment(), and line 67 calls it unconditionally. In production this means the middleware runs twice. Remove one of the two calls.
Proposed fix — remove the unconditional call
-app.UseHttpsRedirection();
-
app.UseStaticFiles();Also applies to: 67-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Program.cs` around lines 42 - 45, The code registers UseHttpsRedirection
middleware twice in non-development mode: once guarded by
app.Environment.IsDevelopment() (negated) and once unconditionally; remove the
duplicate by deleting the unconditional call to app.UseHttpsRedirection() so the
middleware is only added when !app.Environment.IsDevelopment(); locate the
unconditional app.UseHttpsRedirection() invocation (the second occurrence) and
remove it, leaving the existing guarded block (if
(!app.Environment.IsDevelopment()) { app.UseHttpsRedirection(); }) intact.
| using (var scope = app.Services.CreateScope()) | ||
| { | ||
| var services = scope.ServiceProvider; | ||
| var context = services.GetRequiredService<CarpentryShopIdentityDbContext>(); | ||
| context.Database.EnsureCreated(); | ||
| // DbInitializer.Initialize(context); | ||
| } |
There was a problem hiding this comment.
EnsureCreated() conflicts with EF Core migrations.
Database.EnsureCreated() creates the schema without recording anything in the migration history table. If it runs before migrations, subsequent dotnet ef database update calls will fail because the tables already exist but __EFMigrationsHistory is empty. This directly conflicts with the migrations added in this PR and the justfile db-update/migrate commands.
Replace with Database.Migrate() if you want auto-migration at startup, or remove this block entirely and rely on the justfile's db-update recipe to apply migrations explicitly.
Proposed fix
using (var scope = app.Services.CreateScope())
{
var services = scope.ServiceProvider;
var context = services.GetRequiredService<CarpentryShopIdentityDbContext>();
- context.Database.EnsureCreated();
+ context.Database.Migrate();
// DbInitializer.Initialize(context);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Program.cs` around lines 59 - 65, The startup code is calling
Database.EnsureCreated() which conflicts with EF Core migrations; update the
scope block that obtains CarpentryShopIdentityDbContext via
app.Services.CreateScope() to either remove the EnsureCreated call entirely (so
the justfile/db-update handles migrations) or replace
context.Database.EnsureCreated() with context.Database.Migrate() to apply
migrations at startup; ensure you keep the same scope/service resolution
(CreateScope, GetRequiredService<CarpentryShopIdentityDbContext>) and only
change the Database.* call accordingly.
Amp-Thread-ID: https://ampcode.com/threads/T-019caff2-c1f7-76b9-bdc1-61cec128aa3b Co-authored-by: Amp <amp@ampcode.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
CarpentryShop.csproj (2)
23-24: Remove unusedNpgsqlpackage reference.
Npgsql.EntityFrameworkCore.PostgreSQLincludesNpgsqlas a transitive dependency. No direct usage of Npgsql APIs was found in the codebase—all references are toNpgsql.EntityFrameworkCore.PostgreSQL.Metadatain migration files. Remove the direct reference toNpgsqlto avoid unnecessary dependency bloat.Cleanup
- <PackageReference Include="Npgsql" Version="10.0.0" /> <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="10.0.0" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CarpentryShop.csproj` around lines 23 - 24, Remove the unnecessary direct PackageReference to Npgsql (the line with PackageReference Include="Npgsql" Version="10.0.0") from the project file because Npgsql is already provided transitively by Npgsql.EntityFrameworkCore.PostgreSQL; keep the existing PackageReference to Npgsql.EntityFrameworkCore.PostgreSQL (Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="10.0.0") and run a restore/build to verify nothing else directly depends on Npgsql.
21-22: Mark dev-only tooling packages as private assets.At lines 21–22, both
Microsoft.EntityFrameworkCore.ToolsandMicrosoft.VisualStudio.Web.CodeGeneration.Designare design-time-only packages. AddingPrivateAssets=allprevents them from flowing into runtime or publish dependency graphs—a recommended practice for .NET 10 ASP.NET Core projects.Proposed change
- <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="10.0.3" /> - <PackageReference Include="Microsoft.VisualStudio.Web.CodeGeneration.Design" Version="10.0.2" /> + <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="10.0.3"> + <PrivateAssets>all</PrivateAssets> + <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> + </PackageReference> + <PackageReference Include="Microsoft.VisualStudio.Web.CodeGeneration.Design" Version="10.0.2"> + <PrivateAssets>all</PrivateAssets> + <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> + </PackageReference>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CarpentryShop.csproj` around lines 21 - 22, Update the two design-time package references so they don't flow into runtime/publish graphs: modify the PackageReference entries for Microsoft.EntityFrameworkCore.Tools and Microsoft.VisualStudio.Web.CodeGeneration.Design to include PrivateAssets="all" (and optionally IncludeAssets/ExcludeAssets if desired) so they remain dev-only; locate and change the PackageReference elements named Microsoft.EntityFrameworkCore.Tools and Microsoft.VisualStudio.Web.CodeGeneration.Design to add the PrivateAssets attribute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CarpentryShop.csproj`:
- Around line 3-6: Update CI and container SDK versions to match the project
TargetFramework value net10.0: change the workflow variable named dotnet-version
from 8.0.x to 10.0.x and update the Docker base image tag from
mcr.microsoft.com/dotnet/sdk:8.0 to mcr.microsoft.com/dotnet/sdk:10.0. In the
project file remove the explicit Npgsql PackageReference (the EF provider
supplies it transitively) and mark the tooling PackageReference entries for
Microsoft.EntityFrameworkCore.Tools and
Microsoft.VisualStudio.Web.CodeGeneration.Design with PrivateAssets="all" so
they do not flow as transitive dependencies.
---
Nitpick comments:
In `@CarpentryShop.csproj`:
- Around line 23-24: Remove the unnecessary direct PackageReference to Npgsql
(the line with PackageReference Include="Npgsql" Version="10.0.0") from the
project file because Npgsql is already provided transitively by
Npgsql.EntityFrameworkCore.PostgreSQL; keep the existing PackageReference to
Npgsql.EntityFrameworkCore.PostgreSQL
(Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="10.0.0") and run a
restore/build to verify nothing else directly depends on Npgsql.
- Around line 21-22: Update the two design-time package references so they don't
flow into runtime/publish graphs: modify the PackageReference entries for
Microsoft.EntityFrameworkCore.Tools and
Microsoft.VisualStudio.Web.CodeGeneration.Design to include PrivateAssets="all"
(and optionally IncludeAssets/ExcludeAssets if desired) so they remain dev-only;
locate and change the PackageReference elements named
Microsoft.EntityFrameworkCore.Tools and
Microsoft.VisualStudio.Web.CodeGeneration.Design to add the PrivateAssets
attribute.
| <TargetFramework>net10.0</TargetFramework> | ||
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <RuntimeIdentifier>linux-x64</RuntimeIdentifier> <!-- <!– This line is the temp fix –> --> | ||
| <RuntimeIdentifier>linux-x64</RuntimeIdentifier> | ||
| <AllowMissingPrunePackageData>true</AllowMissingPrunePackageData> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Target frameworks =="
rg -nP --glob '*.csproj' '<TargetFramework>[^<]+</TargetFramework>|<TargetFrameworks>[^<]+</TargetFrameworks>'
echo "== global.json =="
fd -HI '^global\.json$' -x sh -c 'echo "--- {} ---"; cat "{}"'
echo "== CI dotnet setup =="
rg -n --glob '*.yml' --glob '*.yaml' 'setup-dotnet|dotnet-version|10\.0|net10\.0'Repository: simanga-dev/carpentry-shop
Length of output: 203
🏁 Script executed:
# Check for direct Npgsql API usage
echo "== Searching for Npgsql usage in codebase =="
rg -n 'using Npgsql|Npgsql\.' --type cs -A 2Repository: simanga-dev/carpentry-shop
Length of output: 999
🏁 Script executed:
# List all files to understand structure
echo "== Repository structure =="
git ls-files | head -20Repository: simanga-dev/carpentry-shop
Length of output: 816
🏁 Script executed:
cat -n .github/workflows/dotnet.ymlRepository: simanga-dev/carpentry-shop
Length of output: 1528
🏁 Script executed:
cat -n DockerfileRepository: simanga-dev/carpentry-shop
Length of output: 605
Update CI and Docker SDK to .NET 10 to match target framework.
The project targets net10.0 but CI workflow and Dockerfile are pinned to SDK 8.0. Build and restore will fail:
.github/workflows/dotnet.ymlline 20: changedotnet-version: 8.0.x→10.0.xDockerfileline 3: changemcr.microsoft.com/dotnet/sdk:8.0→mcr.microsoft.com/dotnet/sdk:10.0
Additionally, remove the explicit Npgsql package reference (line 23 in CarpentryShop.csproj) — the provider package brings it transitively and only EFCore metadata imports are used in the codebase. Mark dev-time tooling packages (Microsoft.EntityFrameworkCore.Tools, Microsoft.VisualStudio.Web.CodeGeneration.Design) as PrivateAssets="all" to prevent transitive dependency leakage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CarpentryShop.csproj` around lines 3 - 6, Update CI and container SDK
versions to match the project TargetFramework value net10.0: change the workflow
variable named dotnet-version from 8.0.x to 10.0.x and update the Docker base
image tag from mcr.microsoft.com/dotnet/sdk:8.0 to
mcr.microsoft.com/dotnet/sdk:10.0. In the project file remove the explicit
Npgsql PackageReference (the EF provider supplies it transitively) and mark the
tooling PackageReference entries for Microsoft.EntityFrameworkCore.Tools and
Microsoft.VisualStudio.Web.CodeGeneration.Design with PrivateAssets="all" so
they do not flow as transitive dependencies.
1 participant
Summary by CodeRabbit
Chores
Infrastructure