Repository navigation
(MAJOR): Target net10.0 only - #140
Merged
Merged
Conversation
Move every project from net8.0 to net10.0, so the repository has a single target framework. .NET 8 reaches end of support in November 2026, and PTrampert.SimplePatch.OpenApi was already net10.0-only. - Bump System.CodeDom to 10.0.12 and drop the System.Text.Json package reference, which is part of the net10.0 shared framework. - Remove a no-op ArgumentNullException.ThrowIfNull on the Optional<T> struct, which the net10.0 analyzers flag as CA2264. - Update README (new Requirements section), docs and AGENTS.md. Closes #76 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PaulTrampert
force-pushed
the
feature/76-target-net10
branch
from
October 4, 2026 21:30
21c6e26 to
444ef8e
Compare
✅ PR Title Formatted CorrectlyThe title of this PR has been updated to match the correct format. Thank you! |
PaulTrampert
added a commit
that referenced
this pull request
Oct 4, 2026
…make PatchClassBuilder static (#141) ## Cause #133 made `PatchClassBuilder` a facade over the internal `RoslynPatchClassBuilder`, and #95 added `UseExperimentalDynamicClassBuilder`, which the facade's `GetPatchClassFor` read on every call. Both kept `PatchClassBuilder.Instance` typed as `PatchClassBuilder`, because changing a property's type breaks binary compatibility. This is the 2.0 change that #135 deferred. Once `Instance` hands out the selected internal builder, nothing hands out a `PatchClassBuilder` any more. Its instance surface (the obsolete constructor, instance `GetPatchClassFor`, and its `IPatchClassBuilder` implementation) only forwards to `Instance` and is dead weight (#143). The two changes are one PR because a static class can't be a property type, so #143 can't compile without #135. ## Fix **Retype `Instance` (#135)** - `public static IPatchClassBuilder Instance` returns the selected builder itself: `EmitPatchClassBuilder.Instance` when the flag is on, otherwise `RoslynPatchClassBuilder.Instance`. Both internal builders already had static `Instance` singletons. - `Instance` is a computed property (`=>`), not an initialised one, so it honours the flag at read time. The flag is a settable static, and the tests flip it back and forth. - Call sites (`PatchJsonConverterFactory`, the Swashbuckle filter, the OpenAPI transformer) already call `PatchClassBuilder.Instance.GetPatchClassFor(...)` on each use, so they compile and behave unchanged against the new type. None of them caches the builder. **Make `PatchClassBuilder` static (#143)** - `public static class PatchClassBuilder` now holds only `Instance` and `UseExperimentalDynamicClassBuilder`. - The constructor, instance `GetPatchClassFor` and the `IPatchClassBuilder` implementation are removed. - The XML docs worth keeping moved. The description of the generated class (its properties, its `Patch` method, sealed and public, the `.Optionals` namespace) is now in the class remarks. The `NotSupportedException` conditions are now in `Instance`'s remarks. `IPatchClassBuilder.GetPatchClassFor` keeps its builder-neutral contract. `RoslynPatchClassBuilder.GetPatchClassFor` had `<inheritdoc cref="PatchClassBuilder.GetPatchClassFor"/>` and now has its own summary and exception docs. - `RoslynPatchClassBuilderTest.GetPatchClassFor_SharesGeneratedTypesAcrossBuilders` constructed `PatchClassBuilder` twice. It is now `GetPatchClassFor_CachesTheGeneratedType`, which calls `RoslynPatchClassBuilder.Instance` twice and still checks that `PatchClassBuilder.Instance` agrees with it while the flag is off. The internal builders have private constructors, so a second instance can't be constructed. **Docs:** `README.md` and `docs/getting-started.md` note that `Instance` returns the selected builder, so you should read it where you use it and not keep it. `AGENTS.md` describes the static class. **Breaking:** - Binaries compiled against `PatchClassBuilder PatchClassBuilder.Instance { get; }` fail with `MissingMethodException`. - `new PatchClassBuilder()` (obsolete since 1.x) and instance `GetPatchClassFor` are gone. - `PatchClassBuilder` no longer implements `IPatchClassBuilder`, and can't be used as a variable, parameter or generic argument type. ## Coordination with sibling PRs into `release/2.0` - **#139 (#75) makes the constructor internal.** This PR deletes the constructor, which supersedes that change. When the two meet, resolve the conflict by deleting the constructor. - **#142 (#126)** makes Emit the only runtime builder and removes the flag. Once it lands, `Instance` collapses to `EmitPatchClassBuilder.Instance`. - **#140 (#76)** changes TFMs. They are unchanged here. ## Alternatives rejected - **Initialise `Instance` once (`{ get; } = ...`).** This would freeze whatever builder the flag selected at type initialisation and ignore later changes to the flag. - **Keep returning the facade, typed as the interface.** This would avoid the read-time caveat, but it keeps the per-call forwarding that #135 asks to remove. It also leaves `Instance` as the one thing that hands out a `PatchClassBuilder`. - **Keep `PatchClassBuilder` non-static, with the constructor internal (#75 alone).** Nothing would construct it, so its instance members would be unreachable dead code. - **Ship #143 as a separate PR stacked on this one.** The maintainer chose to fold it in, because #143 can't compile without the retype. ## Tests New tests in `UseExperimentalDynamicClassBuilderTest`: - `Instance_IsTheRoslynBuilderWhenOff` - `Instance_IsTheEmitBuilderWhenOn` - `Instance_FollowsTheFlagWhenItIsTurnedBackOff` Against the old `PatchClassBuilder.cs`, the NUnit analyzer rejects all three with NUnit2020 (a `SameAs` that always fails because the types are mutually exclusive), so they fail before the fix. CI's build pipeline doesn't run on PRs that target a branch other than `main`, so these results are local only: - `dotnet build`: 0 errors, 21 warnings, the same count as the base commit. - `dotnet test`: all passed. Core 132/132, Swashbuckle 18/18, OpenApi 12/12. The net8.0 test hosts ran with `DOTNET_ROLL_FORWARD=Major`, because only the .NET 10 runtime is installed locally. Closes #135 Closes #143 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Merged
PaulTrampert
added a commit
that referenced
this pull request
Oct 4, 2026
Ships the 2.0 major release from `release/2.0`. > [!IMPORTANT] > Merge with **Rebase and merge**, never squash. Release notes are built from the commit subjects since `v1.5.0`, so a rebase merge gives each PR below its own line. A squash would collapse them into one. ## Included - #139 Make `PatchClassBuilder`'s constructor internal (closes #75) - #140 Target net10.0 only (closes #76) - #141 Retype `PatchClassBuilder.Instance` to `IPatchClassBuilder` and make `PatchClassBuilder` static (closes #135, closes #143) - #142 Make Reflection.Emit the only runtime patch class builder (closes #126) ## Breaking changes - **.NET 10 or later only.** .NET 8 and 9 consumers stay on 1.x. - **`PatchClassBuilder` is a static class.** `PatchClassBuilder.Instance` is typed `IPatchClassBuilder`, and the constructor is gone. - **`UseExperimentalDynamicClassBuilder` is removed.** Reflection.Emit is the only runtime builder. Internal write models are supported when their assembly declares `[InternalsVisibleTo("PTrampert.SimplePatch.Emitted")]`. The Roslyn builder is kept but unused at runtime, pending #144. ## Branch state `release/2.0` has no commits behind `main` and no merge commits. All four commits are `(MAJOR)`, so the version calculation produces 2.0.0. Postponed until after 2.0: #96 and #102, with open questions noted on each. ## Tests Each included PR passed the CI build and tests against `release/2.0`. This PR's build runs the full suite on the combined branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cause
The repository straddled two target frameworks: the core, Schema, Swashbuckle, Sample and most test projects were
net8.0, whilePTrampert.SimplePatch.OpenApi(and its tests) werenet10.0becauseOpenApiSchemaTransformerContext.GetOrCreateSchemaAsynconly exists from .NET 10. .NET 8 reaches end of support in November 2026.Fix
Test.Externaland the Sample, now targetsnet10.0only.System.CodeDom9.0.8 → 10.0.12, the framework-aligned version for the new TFM.System.Text.Jsonpackage reference: it is in the net10.0 shared framework, so the package was redundant.ArgumentNullException.ThrowIfNull(left)inOptional<T>.operator ==.Optional<T>is arecord struct, so boxing it never gives null and the call never threw. I removed it, so behaviour is unchanged.#if NET8_0-style conditionals existed, so there were none to remove.dotnet-library.ymluses the shared workflow, which reads the SDK fromglobal.json(already 10.0.x). Nothing in.github/workflows/assumes a TFM.docs/getting-started.mdloses its ".NET 10 and later" qualifier too. The OpenAPI proposal is a design record, so I left its §3/§4 tables alone and added a note that they describe 1.x. The AGENTS.md Layout table drops its Target column and now says that everything targets net10.0.Breaking: consumers on .NET 8 or .NET 9 can't take 2.x, including
PTrampert.SimplePatch.Swashbuckle, which today gives them full OpenAPI support.Alternatives rejected
net8.0;net10.0. This would keep .NET 8/9 consumers on new releases, but it doubles the build and test matrix for a runtime that is weeks from end of support. It also keeps the OpenApi package an asymmetric exception. The issue's goal is one TFM.Open question for the maintainer
The issue asks whether the net8.0 line gets a maintenance branch (for example
release/1.x) for security fixes until .NET 8 reaches end of support. This PR doesn't create one. That's your call.Test results
dotnet buildgave 0 errors and 21 warnings, the same unique warning set asrelease/2.0(14 distinct warnings, compared before and after with a clean--no-incrementalbuild).dotnet test, all on net10.0:Closes #76
🤖 Generated with Claude Code