Skip to content

Restore compatibility with .NET 9.0 #208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
quis wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Restore compatibility with .NET 9.0 #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc8fe31
Restore compatibility with .NET 9.0
quis Mar 20, 2026
35f4f1e
Add changelog for version 8.0.0
quis Mar 19, 2026
be8f674
Release version 9.0.0
quis Mar 20, 2026
2de7a9b
Tell Appveyor to install .NET 10 before running the tests
quis Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## [9.0.0] - 2026-03-20

* Restore support for .NET Core version 9.0 (supported versions are now 9.0 and 10.0)

## [8.0.0] - (unreleased)

* Updates versions of JWT and Newtonsoft.JSON dependencies
* Drops support for .NET Core version 9.0 or earlier
* Drops support for .NET Framework

Comment thread
quis marked this conversation as resolved.
## [7.2.0] - 2024-07-31
* Added fields related to cost data in response:
* `is_cost_data_ready`: This field is true if cost data is ready, and false if it isn't (Boolean).
Expand Down
8 changes: 4 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ help:

.PHONY: build
build: ## Build project
dotnet build -f=net10.0
dotnet build

.PHONY: format
format:
Expand All @@ -22,15 +22,15 @@ test: single-test

.PHONY: single-test
single-test: build ## run a single test. usage: "make single-test test=[test name]"
dotnet test ./src/GovukNotify.Tests/GovukNotify.Tests.csproj -f=net10.0 --no-build -v=n --filter $(test)
dotnet test ./src/GovukNotify.Tests/GovukNotify.Tests.csproj --no-build -v=n --filter $(test)

.PHONY: build-release
build-release: ## Build release version
dotnet build -c=Release -f=net10.0
dotnet build -c=Release

.PHONY: build-package
build-package: build-release ## Build and package NuGet
dotnet pack -c=Release ./src/GovukNotify/GovukNotify.csproj /p:TargetFrameworks=net8,net9,net10 -o=publish
dotnet pack -c=Release ./src/GovukNotify/GovukNotify.csproj -o=publish
Comment on lines 28 to +33
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the projects now multi-targeting net9.0;net10.0, running dotnet build/dotnet pack without specifying a framework will attempt to build/pack all TFMs. This will break environments that don’t have a .NET 10 SDK available (which is the motivation for this PR). Consider adding a Makefile variable (e.g., TFM?=net9.0) to allow building/packing just net9.0 on constrained CI, or ensuring CI installs/pins the required SDK so the default targets work everywhere.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@quis quis Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. We want to build all TFMs


.PHONY: bootstrap-with-docker
bootstrap-with-docker: ## Prepare the Docker builder image
Expand Down
6 changes: 6 additions & 0 deletions appveyor.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
install:
- ps: |
Invoke-WebRequest -Uri 'https://dot.net/v1/dotnet-install.ps1' -UseBasicParsing -OutFile "$env:temp\dotnet-install.ps1"
& $env:temp\dotnet-install.ps1 -Architecture x64 -Version '10.0.100' -InstallDir "$env:ProgramFiles\dotnet"
clone_script:
- cmd: >-
git clone -q --branch=main https://github.com/alphagov/notifications-net-client.git C:\projects\notifications-net-client
Expand Down Expand Up @@ -31,6 +35,8 @@ environment:
pullId: 0
build: off
image: Visual Studio 2022
before_test:
- cmd: dotnet --version
test_script: dotnet test C:\projects\notifications-net-client\src\GovukNotify.Tests -c=Release
on_success:
- publish.cmd
4 changes: 2 additions & 2 deletions src/GovukNotify.Tests/GovukNotify.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>net10.0</TargetFrameworks>

<TargetFrameworks>net10.0;net9.0</TargetFrameworks>
<LangVersion>14.0</LangVersion>
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting LangVersion to 14.0 will require a compiler/SDK that understands C# 14. If CI/AppVeyor is limited to the .NET 9 SDK (as noted in the PR description), this is likely to fail at compile time with an invalid language version. Prefer removing the override, or making it conditional on the installed SDK version; if specific tests depend on newer syntax, it would be safer to rewrite those assertions to be C# 13-compatible.

Suggested change
<LangVersion>14.0</LangVersion>

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@quis quis Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s fine, this is test code so we’re not shipping it to consumers

<IsPackable>false</IsPackable>
</PropertyGroup>

Expand Down
8 changes: 2 additions & 6 deletions src/GovukNotify/GovukNotify.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

<PropertyGroup>
<RestoreProjectStyle>PackageReference</RestoreProjectStyle>
<TargetFrameworks>net10.0</TargetFrameworks>
<Version>8.0.0</Version>
<TargetFrameworks>net10.0;net9.0</TargetFrameworks>
<Version>9.0.0</Version>
Comment on lines +5 to +6
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TargetFrameworks now includes net10.0, but the repo’s AppVeyor pipeline runs dotnet test/dotnet pack without installing a .NET 10 SDK (see appveyor.txt / publish.cmd). On a machine with only .NET 9 (or earlier) installed this will fail with NETSDK1045 (cannot target .NET 10). Consider making net10.0 conditional on the SDK version/CI environment, or installing/pinning a .NET 10 SDK via global.json/CI setup, so AppVeyor can still build/test/pack.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@quis quis Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2de7a9b

<Authors>GOV.UK Notify</Authors>
<Description>GOV.UK .NET Notify Client</Description>
<PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
Expand All @@ -20,8 +20,4 @@
<PackageReference Include="Newtonsoft.Json" Version="[13.0.4,14]" />
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'net462' ">
<PackageReference Include="System.Net.Http" Version="4.3.4" />
</ItemGroup>

</Project>
Loading