Skip to content
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.

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

Drop netcore2.1 as a target #3986

Merged
merged 1 commit into from
Nov 24, 2021
Merged

Conversation

manfred-brands
Copy link
Member

Fixes #3980

@manfred-brands
Copy link
Member Author

The macOS failure on XML files seems unrelated.

@manfred-brands manfred-brands marked this pull request as ready for review November 11, 2021 10:14
Copy link
Contributor

@jnm2 jnm2 left a comment

Choose a reason for hiding this comment

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

It could be convenient to do #3984 at the same time since the overall number of things would remain the same.

@manfred-brands
Copy link
Member Author

In light of @CharliePoole's comments, I drop this. Replaced with #3988 which add .net6.
That PR also suppresses 'out of support' warnings.

@CharliePoole
Copy link
Contributor

Looking at #3988, I think you may have misunderstood me. My point was that I think you need to keep the out of support second-level test assemblies, even though you are dropping 2.1 support from the framework itself. If I'm not mistaken, dropping the actual support is a decision @rprouse has already made.

@manfred-brands
Copy link
Member Author

The framework itself is targeting net45 and netstandard2.0 and doesn't need dropping anything.
I kept netcoreapp2.1 in the actual tested assembly: nunit.framework.tests which is the only assembly actually tested.
All other assemblies are referenced by this one and only need targeting the same as the framework itself.

The only place I wasn't sure was nunitlite where I left netcoreapp2.1 unless someone tells me it can go from there.
But as I'm new to nunit internals, I might have it wrong.

@CharliePoole
Copy link
Contributor

OK, I'd say wait for Rob to to tell you what he wants to do. I thought that he wanted them all gone and I was only arguing for keeping the second-level tests - those that are run by your own tests - in order to be able to prove that a user with a .NET Core 2.1 test can still use NUnit. IMO it's insufficient to just state that analytically. Ya gotta test it!

All that said, I'm not a team member and it's just one person's opinion. :-)

@rprouse
Copy link
Member

rprouse commented Nov 14, 2021

I'd like to drop support for compiling test assemblies to 2.1, so I'd like to see this reopened.

@manfred-brands
Copy link
Member Author

Rebased and also removed installing netcore 2.1 from github actions.

Copy link
Member

@rprouse rprouse left a comment

Choose a reason for hiding this comment

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

@rprouse rprouse merged commit 63dadc2 into nunit:master Nov 24, 2021
@manfred-brands manfred-brands deleted the issue/3980 branch February 16, 2022 23:39
rprouse added a commit that referenced this pull request Mar 2, 2022
Drop netcore2.1 as a target (backport #3986)
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.

Drop netcoreapp2.1 targets
4 participants