Skip to content

Add a .NET 8.0 target#4884

Merged
manfred-brands merged 3 commits intonunit:mainfrom
manfred-brands:net8-binaries
Nov 26, 2024
Merged

Add a .NET 8.0 target#4884
manfred-brands merged 3 commits intonunit:mainfrom
manfred-brands:net8-binaries

Conversation

@manfred-brands
Copy link
Member

@manfred-brands manfred-brands commented Nov 13, 2024

Fixes #4890
Works toward #4857

This adds the .NET 8.0 target. So that if users that are not allowed to use end-of-life frameworks due to lack of security updates can the .NET8.0 binaries.

Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

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

Looks good - only comments and questions from me 👍

Comment on lines +43 to +46
/*
/// <summary>Test fixture not found - No longer in use</summary>
//public const int FIXTURE_NOT_FOUND = -3;
*/
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should rather remove this ?

Comment on lines +6 to +8
#if !NET8_0_OR_GREATER
using System.Runtime.Serialization;
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Just suggestions. Should we move the usings outside the namespace? (and perhaps move System.Runtime.Serialization down to the sole purpose, so we avoid having two !NET8_0_OR_GREATER blocks ?

EDIT: I can see that we do it differently in many of the ...Exception files, so I'm also happy with the current solution (to keep the fix as small as possible)

Comment on lines +139 to +141
#if !NET6_0_OR_GREATER
using System.Security.Permissions;
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Why is this need now ?

Comment on lines +586 to 587
#if !NET8_0_OR_GREATER
#if !NET6_0_OR_GREATER
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't #if !NET6_0_OR_GREATER imply #if !NET8_0_OR_GREATER ?
Or is this to to anticipate when we remove the .NET6 target ?

Copy link
Member

@OsirisTerje OsirisTerje left a comment

Choose a reason for hiding this comment

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

We need to add the .net 8 bits to the nunit.nuspec file, otherwise the bits don't go into the nupkg.

@manfred-brands
Copy link
Member Author

We need to add the .net 8 bits to the nunit.nuspec file, otherwise the bits don't go into the nupkg.

Thanks @OsirisTerje

I forgot and to ensure nobody will forget it again, I added a nunit test to verify proper file references in .nuspec files.
Similar to the one you added for dependencies.

The bulk of the size is the 3 copies of the .xml files 1.2MB per target.
They are 99% the same with a few TFM specific APIs.
I tired to put only one of the in lib/any but it then doesn't seem to be picked up.
Searching the net didn't result in any solutions.

@OsirisTerje
Copy link
Member

Lgtm, @manfred-brands : can we merge ?

@manfred-brands manfred-brands merged commit 0c1ef7f into nunit:main Nov 26, 2024
@manfred-brands manfred-brands deleted the net8-binaries branch November 26, 2024 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add .net 8 as a target framework in the package

3 participants