-
Notifications
You must be signed in to change notification settings - Fork 547
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
Switch to embedded PackageIcon #1286
Conversation
That's really up to you at this point. NuGet.org will eventually transition to a world where it only accepts packages with a self contained icon, but other feeds may not implement this soon if ever. //cc @karann-msft @dominoFire |
I'm not sure this answers the question we had. For NuGet.org, should the .nuspec reference the icon file? |
The newly icon metadata has to be used for NuGet.org. |
@nkolev92 what do you mean by this? I followed the example from https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#packing-an-icon-image-file in this PR, which resulted in an icon file in the root of the .nupkg, but no reference to it in the |
The adding of the propery should burn it into the nuspec. <PackageIcon>icon.png</PackageIcon> If that's not the case, make sure a high enough version of the SDK is being sued. |
Ah, looks like you're correct. I made this change in several repos, all of which auto-generate nuspec's by default, except for a few exceptions (such as the manual ones in this PR). The ones which were auto-generated do have the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wtgodbe please let me know when the missing property is in the .nuspec files and you've confirmed the final packages
With the last change, this now includes the icon metadata in all .nuspec's, including the manually generated ones - @dougbu PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but should target 'release/6,.3'
Closing in favor of #1291 |
Part of https://github.com/aspnet/AspNetCore-Internal/issues/3146
Local build passes the smell test with these changes. I'm still unsure on whether or not we need to include a reference to the package icon in the
.nuspec
- the "old way" did so automatically, the "new way" does not (https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#packing-an-icon-image-file). CC @tmat who may know more about that question.