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

Support ASP.NET Core PackageReference when targeting .NET Core 3.0 or higher #2738

Merged

Conversation

dsplaisted
Copy link
Member

Fixes dotnet/cli#10124

Also fixes #2527

@dsplaisted dsplaisted requested review from nguerrera, livarcocc and a team December 10, 2018 01:29
@dsplaisted dsplaisted force-pushed the 10124-versionless-packagereference branch from e6ffce2 to c6be6bc Compare December 10, 2018 05:50
@livarcocc livarcocc added this to the 3.0.1xx milestone Dec 10, 2018
@dsplaisted
Copy link
Member Author

@DamianEdwards @natemcmaster Would you (or others) like to review the experience and message text here?

Damian had commented:

Can't we support it but make it a warning?

This PR adds the following warning if you have a PackageReference to Microsoft.AspNetCore.App (with any Version, or no Version):

NETSDK1080: A PackageReference to Microsoft.AspNetCore.App is not necessary when targeting .NET Core 3.0 or higher. If Microsoft.NET.Sdk.Web is used, the shared framework will be referenced automatically. Otherwise, the PackageReference should be replaced with a FrameworkReference.

If you have a PackageReference to Microsoft.AspNetCore.All, you will get the following error:

NETSDK1079: The Microsoft.AspNetCore.All package is not supported when targeting .NET Core 3.0 or higher. A FrameworkReference to Microsoft.AspNetCore.App should be used instead, and will be implicitly included by Microsoft.NET.Sdk.Web.

How does this look?

@DamianEdwards
Copy link
Member

Looks good.

<Target Name="FixAspNetReference" AfterTargets="ResolveFrameworkReferences">
<ItemGroup>
<PackageReference>
<Publish Condition="'%(Identity)' == 'Microsoft.AspNetCore.App'">true</Publish>
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it come out out ResolveFrameworkReferences this way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out it does, so this was unnecessary. I think this was a leftover change from previous work I was doing.

Copy link
Contributor

@natemcmaster natemcmaster left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing :)

@AraHaan
Copy link
Member

AraHaan commented Oct 10, 2021

What if you wanted to use this with your own framework and wanted to use the KnownFrameworkReference to add a framework to the known ones and do not explicitly set the things added in this PR?

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