Skip to content

Updates dependencies. Removes ASP.NET support for netstandard2.0 & tests for netcoreapp2.1 #267

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

Jump to bottom
Merged
merged 7 commits into from
Oct 5, 2023
Merged

Updates dependencies. Removes ASP.NET support for netstandard2.0 & tests for netcoreapp2.1 #267

merged 7 commits into from
Oct 5, 2023

Conversation

rossgrambo
Copy link
Contributor

No description provided.

@rossgrambo
Copy link
Contributor Author

I removed netcoreapp2.1 from tests, as FeatureManagement.AspNetCore was no longer able to build with on netcoreapp2.1 after removing netstandard2.0.

I did not remove netstandard2.0 from the normal FeatureManagement package. We could either:

  1. Remove it there as well
  2. Leave it and accept that tests won't run against it
  3. Split the tests between ASP.NET tests and FeatureManagement tests and run on both.

@zhenlan
Copy link
Member

zhenlan commented Sep 26, 2023

I removed netcoreapp2.1 from tests, as FeatureManagement.AspNetCore was no longer able to build with on netcoreapp2.1 after removing netstandard2.0.

Good. We should have dropped netcoreapp2.1 long ago.

I did not remove netstandard2.0 from the normal FeatureManagement package. We could either:

  1. Remove it there as well
  2. Leave it and accept that tests won't run against it
  3. Split the tests between ASP.NET tests and FeatureManagement tests and run on both.

We need to keep Microsoft.FeatureManagement as a netstandard2.0 package. What's the problem with testing?

@rossgrambo
Copy link
Contributor Author

What's the problem with testing?

The testing project has targets as well. If target == netstandard2.0 then the testing project can't build, as it depends on the ASP.NET package (which can't built in netstandard2.0 without the deprecated packages).

If I remove netstandard2.0 from testing, then we aren't running unit tests for that framework.

@jimmyca15
Copy link
Member

What's the problem with testing?

The testing project has targets as well. If target == netstandard2.0 then the testing project can't build, as it depends on the ASP.NET package (which can't built in netstandard2.0 without the deprecated packages).

If I remove netstandard2.0 from testing, then we aren't running unit tests for that framework.

we need to separate the test project into two projects then.

@rossgrambo
Copy link
Contributor Author

Could we instead upgrade to netstandard2.1?

@zhenlan
Copy link
Member

zhenlan commented Sep 27, 2023 •
edited
Loading

What's the problem with testing?

The testing project has targets as well. If target == netstandard2.0 then the testing project can't build, as it depends on the ASP.NET package (which can't built in netstandard2.0 without the deprecated packages).
If I remove netstandard2.0 from testing, then we aren't running unit tests for that framework.

we need to separate the test project into two projects then.

Right, we should separate test projects for MS.FM and MS.FM.ANC.

The test projects are applications. They are not libraries (or designed to be consumed by anyone else). We don't need to build test projects targeting netstandard. That said, you shouldn't have a problem once you separate the test project.

@rossgrambo
Copy link
Contributor Author

Updated this PR to target only supported frameworks and separated the FeatureManagement and FeatureManagement.AspNetCore test packages.

jimmyca15
jimmyca15 reviewed Sep 28, 2023
View reviewed changes
@jimmyca15
Copy link
Member

PR build is failing. The container we use to build doesn't have .net 7 sdk. Seems like we need a new container.

@rossgrambo rossgrambo force-pushed the rossgrambo/dependency-update branch from b58021b to 2e0d4df Compare October 4, 2023 22:25
@rossgrambo rossgrambo force-pushed the rossgrambo/dependency-update branch from 2e0d4df to bab0180 Compare October 4, 2023 22:41
@rossgrambo rossgrambo merged commit a7e5b90 into main Oct 5, 2023
rossgrambo added a commit that referenced this pull request Oct 5, 2023
@rossgrambo
Merge main changes into preview (#271)
3e9179a 
* Adjusts the configuration provider to no longer check top level for feature flags (#261)

* Adjusts the configuration provider to no longer check top level for feature flags

* Adding missing using and removes unused else

* Remove unused string

* Add null check to constructor

* Updates dependencies. Removes ASP.NET support for netstandard2.0 & tests for netcoreapp2.1 (#267)

* Updating dependencies

* Updates packages and resolves issues

* Remove pointless semicolon

* Adjusts dotnet installation for buiddy build

* Removes ASP.NET tests from .NET tests

* Removes unused usings

* Adjust the template type for RazorPages project

* Resolves missing ExternalInit for netstandard2.1
@amerjusupovic amerjusupovic mentioned this pull request Oct 31, 2023
Merged
@rossgrambo rossgrambo deleted the rossgrambo/dependency-update branch January 16, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimmyca15 jimmyca15 jimmyca15 approved these changes

@zhenlan zhenlan Awaiting requested review from zhenlan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rossgrambo @zhenlan @jimmyca15