Skip to content

Remove .NET 3.5 and 4.0 targets - #3760

Merged
rprouse merged 9 commits into
masterfrom
issue/3708
Feb 12, 2021
Merged

Remove .NET 3.5 and 4.0 targets#3760
rprouse merged 9 commits into
masterfrom
issue/3708

Conversation

@rprouse

@rprouse rprouse commented Feb 6, 2021

Copy link
Copy Markdown
Member

Fixes #3708
Fixes #3758

  • Removes .NET 3.5 and 4.0 targets from the build scripts, nuspecs and the project files
  • Remove .NET 3.5 and 4.0 pragmas and code
  • Remove TASK_PARALLEL_LIBRARY_API pragma as it is now supported on all platforms
  • Remove NET45 pragmas that are no longer needed

@rprouse rprouse self-assigned this Feb 6, 2021
@rprouse
rprouse requested review from jnm2, mikkelbu and stevenaw February 6, 2021 16:36
@rprouse

rprouse commented Feb 6, 2021

Copy link
Copy Markdown
Member Author

Sorry for such a large review guys. At least it is mainly just code deletion. 😺

Comment thread build.cake

@stevenaw stevenaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Phew, that was a big review. Thanks for putting this all together @rprouse
Most everything made sense, but I noticed a few things while searching the codebase for 3.5 on your branch that might still be lingering. GitHub doesn't make it easy (to my knowledge) to comment on lines too far from a changed line, but I think there's still a few things that could be removed in RuntimeFramework and PlatformDetectionTests:

EDIT: Though not sure if we care about those within this PR if this is focused just on the compile-time stuff

Comment thread nuget/framework/nunit.nuspec
Comment thread nuget/nunitlite/nunitlite.nuspec
Comment thread src/NUnitFramework/tests/Assertions/AssertThatTests.cs
Comment thread src/NUnitFramework/tests/Internal/PlatformDetectionTests.cs Outdated
@rprouse

rprouse commented Feb 7, 2021

Copy link
Copy Markdown
Member Author

Thanks for a thorough review @stevenaw, I really appreciate it. I've resolved all of your suggestions except the DetectNet35() test. There are many OS and runtime tests in that class that are no longer supported by NUnit, so I plan to do a full pass at the Platform attribute and related classes soon and clean them up in a separate PR.

@jnm2 jnm2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feels good!

Comment thread src/NUnitFramework/framework/Api/NUnitTestAssemblyRunner.cs Outdated
Comment thread src/NUnitFramework/tests/nunit.framework.tests.csproj
@stevenaw

stevenaw commented Feb 9, 2021

Copy link
Copy Markdown
Member

Thanks @rprouse for the quick response. That makes sense about tackling Platform separately

@rprouse

rprouse commented Feb 11, 2021

Copy link
Copy Markdown
Member Author

Damn, the latest builds started failing because .NET 5.0 is not installed on Linux or OSX on Azure. We just removed the install because newer versions were installed on those platforms! 👿

@jnm2

jnm2 commented Feb 11, 2021

Copy link
Copy Markdown
Contributor

@rprouse I've had issues with Azure Pipelines images where simply installing an older runtime causes the preinstalled runtimes to no longer be found, so once I install one, I have to install all. Could that be it?

@rprouse

rprouse commented Feb 12, 2021

Copy link
Copy Markdown
Member Author

@jnm2 we removed the install of 5.0 a few weeks ago for just that reason, a newer version came pre-installed on the build images. Now it appears that it isn't installed anymore!

@rprouse
rprouse merged commit 34e3cd9 into master Feb 12, 2021
@rprouse
rprouse deleted the issue/3708 branch February 12, 2021 03:53
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 .NET 3.5 Build Targets Drop .NET 4.0 Build Target

3 participants