feat(eqxWebCs): Port to C# 14 by bartelink · Pull Request #139 · jet/dotnet-templates

bartelink · 2026-04-07T15:46:27Z

No description provided.

Copilot

Pull request overview

Ports the equinox-web-csharp/ template to a more modern ASP.NET Core + C# style by removing Startup, adopting minimal hosting, enabling nullable/implicit usings, and refreshing dependencies/documentation.

Changes:

Switch to minimal hosting (Program.cs) and extract ServiceBuilder into its own file.
Enable nullable reference types + implicit usings, and refactor domain/web code to newer C# syntax (primary constructors, required members, file-scoped namespaces).
Update several NuGet packages and add repo-local Copilot guidance.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
equinox-web-csharp/Web/Web.csproj	Updates TFM and package references; enables nullable/implicit usings; adds `DefineConstants`.
equinox-web-csharp/Web/Startup.cs	Removes the `Startup`-based hosting model.
equinox-web-csharp/Web/ServiceBuilder.cs	Extracts the former nested `ServiceBuilder` class into its own file.
equinox-web-csharp/Web/Program.cs	Replaces `Startup` with minimal hosting, DI wiring, middleware, and store connection.
equinox-web-csharp/Web/MemoryStoreContext.cs	Modernizes the context class (primary ctor, nullable annotations).
equinox-web-csharp/Web/EventStoreContext.cs	Modernizes connection/cache handling and nullable annotations.
equinox-web-csharp/Web/EquinoxContext.cs	Updates nullable annotations and minor formatting.
equinox-web-csharp/Web/CosmosContext.cs	Modernizes connection logic and nullable annotations.
equinox-web-csharp/Web/Controllers/TodosController.cs	Refactors controller and introduces a custom header model binder for `ClientId`.
equinox-web-csharp/Domain/Todo.cs	Refactors domain model to modern C# (file-scoped ns, required, collection expressions).
equinox-web-csharp/Domain/Infrastructure.cs	Removes unused `using` directives.
equinox-web-csharp/Domain/Domain.csproj	Updates TFM and enables nullable/implicit usings; adjusts package refs.
equinox-web-csharp/Domain/ClientId.cs	Simplifies `ClientId` and removes the `TypeConverter`-based parsing path.
equinox-web-csharp/Domain/Aggregate.cs	Modernizes syntax and formatting.
.github/copilot-instructions.md	Adds repository-specific Copilot guidance and build/test notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-07T15:54:35Z


  <PropertyGroup>
-    <TargetFramework>net6.0</TargetFramework>
+    <TargetFramework>net10.0</TargetFramework>


<TargetFramework>net10.0</TargetFramework> is likely incompatible with the repo’s pinned SDK (global.json uses .NET SDK 8.0.100). This will break template instantiation/build in CI unless the repo/tooling is updated to a .NET 10 SDK. Consider keeping the template at net8.0 (or the repo’s established TFMs) and, if needed, set LangVersion separately for newer C# features.

Suggested change

<TargetFramework>net10.0</TargetFramework>

<TargetFramework>net8.0</TargetFramework>

Copilot · 2026-04-07T15:54:35Z

+    <ImplicitUsings>enable</ImplicitUsings>
    <WarningLevel>5</WarningLevel>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
+    <DefineConstants>todos</DefineConstants>


<DefineConstants>todos</DefineConstants> overrides the SDK defaults (e.g., DEBUG/TRACE) and also forces todos on for every generated project. That conflicts with template.json where todos is an optional parameter and related files are excluded when !todos, risking broken generated outputs. Prefer letting the template engine control optional code (or append via $(DefineConstants);todos behind a template conditional).

Suggested change

<DefineConstants>todos</DefineConstants>

Copilot · 2026-04-07T15:54:36Z


  <PropertyGroup>
-    <TargetFramework>net6.0</TargetFramework>
+    <TargetFramework>net10.0</TargetFramework>


<TargetFramework>net10.0</TargetFramework> is likely incompatible with the repo’s pinned SDK (global.json uses .NET SDK 8.0.100). Unless the repo is moving to a .NET 10 toolchain, this will break template build validation. Consider targeting net8.0 and configuring C# language version explicitly if needed.

Suggested change

<TargetFramework>net10.0</TargetFramework>

<TargetFramework>net8.0</TargetFramework>

Copilot · 2026-04-07T15:54:36Z

+                o.JsonSerializerOptions.Converters.Add(c);
+        });
+//#if todos
+    builder.Services.AddCors();
+//#endif
+
+    var equinoxContext = ConfigureStore();
+    builder.Services.AddSingleton(_ => equinoxContext);
+    builder.Services.AddSingleton(sp => new ServiceBuilder(equinoxContext, Serilog.Log.ForContext<EquinoxContext>()));
+//#if todos
+    builder.Services.AddSingleton(sp => sp.GetRequiredService<ServiceBuilder>().CreateTodoService());
+//#endif
+#if aggregate
+    builder.Services.AddSingleton(sp => sp.GetRequiredService<ServiceBuilder>().CreateAggregateService());
+#endif


The todos blocks here use //#if todos (template-style conditional), but later in this file the CORS middleware uses #if todos (C# preprocessor). Mixing the two is fragile and, combined with <DefineConstants>todos</DefineConstants>, can produce incorrect generated outputs. Use a single mechanism consistently (prefer template conditionals for templates) so --todos reliably includes/excludes all related wiring.

Copilot · 2026-04-07T15:54:36Z

+    app.UseHttpsRedirection()
+       .UseSerilogRequestLogging() // see https://nblumhardt.com/2019/10/serilog-in-aspnetcore-3/
+#if todos
+       // NB Jet does not own, control or audit https://todobackend.com; it is a third party site; please satisfy yourself that this is a safe thing to use in your environment before using it.
+       .UseCors(x => x.WithOrigins("https://www.todobackend.com").AllowAnyHeader().AllowAnyMethod())
+#endif
+       ;
+
+// add controllers from this assembly
+    app.MapControllers();
+    app.MapMetrics(); // Host /metrics for Prometheus


CORS middleware is being added without an explicit UseRouting() in the pipeline. In the previous Startup-based pipeline, UseCors ran after UseRouting; without that, endpoint-based CORS may not apply as intended. Consider adding app.UseRouting() (and placing UseCors after it) or moving CORS to endpoint mapping (e.g., require CORS on the mapped controllers/endpoints).

Copilot · 2026-04-07T15:54:37Z

+        var clientId = string.IsNullOrEmpty(headerValue)
+            ? new ClientId(Guid.Empty)
+            : new ClientId(Guid.Parse(headerValue));
+        bindingContext.Result = ModelBindingResult.Success(clientId);


Guid.Parse(headerValue) will throw for malformed header values, resulting in a 500 rather than a model-binding failure/400. Prefer Guid.TryParse and set a failed binding result (and/or ModelState error) when the header is present but invalid.

Suggested change

var clientId = string.IsNullOrEmpty(headerValue)

? new ClientId(Guid.Empty)

: new ClientId(Guid.Parse(headerValue));

bindingContext.Result = ModelBindingResult.Success(clientId);

if (string.IsNullOrEmpty(headerValue))

{

bindingContext.Result = ModelBindingResult.Success(new ClientId(Guid.Empty));

return Task.CompletedTask;

}

if (!Guid.TryParse(headerValue, out var clientGuid))

{

bindingContext.ModelState.TryAddModelError(

bindingContext.ModelName,

"The COMPLETELY_INSECURE_CLIENT_ID header must be a valid GUID.");

bindingContext.Result = ModelBindingResult.Failed();

return Task.CompletedTask;

}

bindingContext.Result = ModelBindingResult.Success(new ClientId(clientGuid));

Copilot · 2026-04-07T15:54:37Z

+## Key Conventions
+
+- **Target framework**: `net8.0` for EXE projects; `netstandard2.0` for the template package itself.
+- **Test variants**: `DotnetBuild.fs` uses xUnit `TheoryData` classes (e.g. `ProProjector`, `EqxWebs`) to enumerate parameter combinations. DEBUG builds run a reduced subset; Release runs the full matrix.
+- **`scratch-area/`**: Generated by tests; do not commit. Cleared before each test run.


This document states the repo convention is net8.0 for EXE projects, but this PR updates equinox-web-csharp projects to net10.0. Either update the convention text here or keep the template TFMs aligned with the documented convention to avoid confusing contributors.

Copilot AI review requested due to automatic review settings April 7, 2026 15:46

Copilot started reviewing on behalf of bartelink April 7, 2026 15:47 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

bartelink force-pushed the cs14 branch from b28a24a to 3024415 Compare April 8, 2026 06:09

bartelink added 3 commits April 8, 2026 14:33

Port to C# 14

82583e2

Fix, polish codec scheme

717c137

Update to .NET/F# 10

75eef4f

bartelink force-pushed the cs14 branch from 106bba8 to 75eef4f Compare April 8, 2026 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eqxWebCs): Port to C# 14#139

feat(eqxWebCs): Port to C# 14#139
bartelink wants to merge 3 commits into
masterfrom
cs14

bartelink commented Apr 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	<TargetFramework>net10.0</TargetFramework>
	<TargetFramework>net8.0</TargetFramework>

-        var clientId = string.IsNullOrEmpty(headerValue)
-            ? new ClientId(Guid.Empty)
-            : new ClientId(Guid.Parse(headerValue));
-        bindingContext.Result = ModelBindingResult.Success(clientId);
+        if (string.IsNullOrEmpty(headerValue))
+        {
+            bindingContext.Result = ModelBindingResult.Success(new ClientId(Guid.Empty));
+            return Task.CompletedTask;
+        }
+        if (!Guid.TryParse(headerValue, out var clientGuid))
+        {
+            bindingContext.ModelState.TryAddModelError(
+                bindingContext.ModelName,
+                "The COMPLETELY_INSECURE_CLIENT_ID header must be a valid GUID.");
+            bindingContext.Result = ModelBindingResult.Failed();
+            return Task.CompletedTask;
+        }
+        bindingContext.Result = ModelBindingResult.Success(new ClientId(clientGuid));

Conversation

bartelink commented Apr 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants