Skip to content

Add net461 to targetFrameworksList - #1961

Closed
Anipik wants to merge 1 commit into
dotnet:masterfrom
Anipik:461targetFramework
Closed

Add net461 to targetFrameworksList#1961
Anipik wants to merge 1 commit into
dotnet:masterfrom
Anipik:461targetFramework

Conversation

@Anipik

@Anipik Anipik commented Feb 2, 2019

Copy link
Copy Markdown
Contributor

Currently when we use this package in netfx version like 461 , the nuget resolves this version to netstandard.
The conditionalFact attributes need to be cross compiled which is not possible in netstandard , hence the conditionalFact always evaluated the condition to be false.

@Anipik
Anipik requested review from ViktorHofer and safern February 2, 2019 00:24

@safern safern 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.

ML.NET is using net461 to run their tests. So they need a lower version of assets in order to resolve to the right set of dlls from xunit extensions dlls for full framework.

@ViktorHofer can we remove net472 and just cross compile it for net461? I guess there wouldn't cause any issues since the source code is the same.

@ViktorHofer

Copy link
Copy Markdown
Member

can we remove net472 and just cross compile it for net461? I guess there wouldn't cause any issues since the source code is the same.

Sure we can't downgrade the .NET Framework version to net461, that shouldn't cause any issue.

ML.NET is using net461 to run their tests.

I would prefer upgrading their version to net472. Isn't that feasible?

@Anipik

Anipik commented Feb 2, 2019

Copy link
Copy Markdown
Contributor Author

Sure we can't downgrade the .NET Framework version to net461, that shouldn't cause any issue.

did you mean can here ?

@ViktorHofer is there any specific reason for choosing the net472 ?

@ViktorHofer

Copy link
Copy Markdown
Member

Yeah can sorry.

Choosing the highest possible framework version gives us more flexibility but if ML.NET can't support net472 I'm fine with downgrading it to the highest supported one.

@safern

safern commented Feb 5, 2019

Copy link
Copy Markdown
Member

I agree with @ViktorHofer that is best to be in the latest framework. Is there any reason why ML.NET can target net472?

@Anipik

Anipik commented Feb 5, 2019

Copy link
Copy Markdown
Contributor Author

We can close this one as ml people are moving towards not using conditionalFact entirely dotnet/machinelearning#2402

@Anipik Anipik closed this Feb 5, 2019
@Anipik
Anipik deleted the 461targetFramework branch November 23, 2019 00:13
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.

3 participants