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

Switch to embedded PackageIcon #1896

Closed
wants to merge 2 commits into from

Conversation

@wtgodbe
Copy link
Contributor

wtgodbe commented Sep 23, 2019

Part of aspnet/AspNetCore-Internal#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 @nkolev92 who may know more about that question.

@wtgodbe wtgodbe requested a review from aspnet/build Sep 23, 2019
@wtgodbe

This comment has been minimized.

Copy link
Contributor Author

wtgodbe commented Sep 23, 2019

Looks like we don't need to worry about the .nuspec, as per aspnet/EntityFramework6#1286 (comment)

Copy link
Member

dougbu left a comment

@wtgodbe please let me know when the missing property is in the .nuspec files and you've confirmed the final packages

@wtgodbe

This comment has been minimized.

Copy link
Contributor Author

wtgodbe commented Sep 24, 2019

With the last change, this now includes the icon metadata in all .nuspec's, including the manually generated ones - @dougbu PTAL

@dougbu
dougbu approved these changes Sep 24, 2019
Copy link
Member

dougbu left a comment

Looks good but should target 'release/3.0'

@wtgodbe

This comment has been minimized.

Copy link
Contributor Author

wtgodbe commented Sep 24, 2019

Closing in favor of #1897

@wtgodbe wtgodbe closed this Sep 24, 2019
@wtgodbe wtgodbe deleted the Embedded branch Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.