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

Remove netstandard1.6 and netcoreapp1.1 builds #854

Merged
merged 9 commits into from
Jan 16, 2021
Merged

Conversation

CharliePoole
Copy link
Collaborator

Fixes #725

@ChrisMaddock
Copy link
Member

Thanks for doing this Charlie - I'd been meaning to get around to it for a while!

Do you mind if we put it on hold until after the 3.12 release? I'd prefer not to introduce any new major changes between the beta and the release, but I'm hoping to get the release out this weekend.

@CharliePoole
Copy link
Collaborator Author

No sweat! I'm working on some other stuff at the moment anyway.

@CharliePoole
Copy link
Collaborator Author

Rebased on latest master

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.

All of the changes look sound, and I only have a couple of questions/comments. I tried to search for netstandard1.6 and netcoreapp1.1 and I think I found a couple of places that where missed:

  • nunit-console\azure-pipelines.yml tries to publish netcoreapp1.1 test results which are missing (gives a warning on CI)
  • nunit-console\src\NUnitEngine\nunit.engine.tests\Services\AgentProcessTests.cs has a commented TestCase mentioning netcoreapp1.1 - perhaps we should remove this
  • nunit-console\nuget\engine\nunit.engine.nuspec lines 23-30 has a group targeting netstandard1.6 that can be removed

build.cake Outdated Show resolved Hide resolved
src/NUnitEngine/EngineApiVersion.cs Show resolved Hide resolved
@CharliePoole
Copy link
Collaborator Author

All good catches... thanks.

@CharliePoole
Copy link
Collaborator Author

@mikkelbu I don't use Azure but I did the obvious-seeming changes. I'd appreciate it if you'd look at that file.

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.

LGTM and the changes to azure-pipelines.yml seem correct.

@CharliePoole CharliePoole merged commit f929716 into master Jan 16, 2021
@CharliePoole CharliePoole deleted the issue-725 branch January 16, 2021 21:27
@CharliePoole
Copy link
Collaborator Author

Whoops! I didn't mean to merge this one - got confused with the equivalent on the GUI.

@ChrisMaddock Sorry... I know you wanted to hold on this till after the release. Please let me know what you want to do. I can back it out or you can cherry-pick or we can leave it in as you prefer.

@ChrisMaddock
Copy link
Member

ChrisMaddock commented Jan 16, 2021

Hi Charlie,

I'd still prefer to keep this for 3.13, even though we expect usage of .NET Standard 1.6 to be low-to-zero. I've undeleted the issue-725 branch and used GitHub's "revert" functionality to create #864. Would you mind recreated a new pull request from the issue-725 branch, to merge after release? Wouldn't want to put your work under my name! 😄

Thanks for that. Planning to release 3.12 tomorrow, then we'll get this merged in.

@CharliePoole
Copy link
Collaborator Author

Yes... I restored the issue-725 branch but I'll wait to create a new pull request until after the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove .NET Standard 1.6 build
3 participants