Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

chore: Fix deprecated Nuget pcakage elements. Fixes NU5048 and NU5125 warnings#1198

Closed
p-nagpal wants to merge 1 commit intofeature/telephonyfrom
user/p-nagpal/build-fix
Closed

chore: Fix deprecated Nuget pcakage elements. Fixes NU5048 and NU5125 warnings#1198
p-nagpal wants to merge 1 commit intofeature/telephonyfrom
user/p-nagpal/build-fix

Conversation

@p-nagpal
Copy link
Contributor

@p-nagpal p-nagpal commented Jul 7, 2021

Purpose

Fixes the following Nuget warnings when generating the package:

NU5048 The 'PackageIconUrl'/'iconUrl' element is deprecated. Consider using the 'PackageIcon'/'icon' element instead. Learn more at https://aka.ms/deprecateIconUrl

NU5125 The 'licenseUrl' element will be deprecated. Consider using the 'license' element instead.

Changes

Change project file according to docs below:

https://docs.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu5048?f1url=%3FappId%3DDev16IDEF1%26l%3DEN-US%26k%3Dk(NU5048)%26rd%3Dtrue

https://docs.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu5125?f1url=%3FappId%3DDev16IDEF1%26l%3DEN-US%26k%3Dk(NU5125)%26rd%3Dtrue

#minor

Copy link
Contributor

@ryanisgrig ryanisgrig left a comment

Choose a reason for hiding this comment

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

For licenseUrl, I have a PR #1197 out that uses <PackageLicenseExpression>MIT</PackageLicenseExpression> rather than referencing a license file directly. This is the same that is used in the dotnet repo.

For PackageIconUrl/iconUrl, I believe that we need to continue using these as they reference a remote URL and this isn't supported with PackageIcon/icon. If we switch to use the recommended properties we should host the icon.png ourselves rather than pointing to an empty path (as the icon is used in Composer's Package Manager). Not sure if we want to risk having multiple copies of the Bot Builder icon.png rather than all point to the same location. The dotnet repo is still using the url, for reference.

@p-nagpal
Copy link
Contributor Author

p-nagpal commented Jul 8, 2021

For licenseUrl, I have a PR #1197 out that uses <PackageLicenseExpression>MIT</PackageLicenseExpression> rather than referencing a license file directly. This is the same that is used in the dotnet repo.

For PackageIconUrl/iconUrl, I believe that we need to continue using these as they reference a remote URL and this isn't supported with PackageIcon/icon. If we switch to use the recommended properties we should host the icon.png ourselves rather than pointing to an empty path (as the icon is used in Composer's Package Manager). Not sure if we want to risk having multiple copies of the Bot Builder icon.png rather than all point to the same location. The dotnet repo is still using the url, for reference.

Thanks @ryanlengel . In that case, I can merge in your PR for License. For IconUrl, is it alright to ignore NU5048 since we cannot change the file location?

@carlosscastro
Copy link
Member

Thanks for the input @ryanlengel !!!!

@ryanisgrig
Copy link
Contributor

For licenseUrl, I have a PR #1197 out that uses <PackageLicenseExpression>MIT</PackageLicenseExpression> rather than referencing a license file directly. This is the same that is used in the dotnet repo.
For PackageIconUrl/iconUrl, I believe that we need to continue using these as they reference a remote URL and this isn't supported with PackageIcon/icon. If we switch to use the recommended properties we should host the icon.png ourselves rather than pointing to an empty path (as the icon is used in Composer's Package Manager). Not sure if we want to risk having multiple copies of the Bot Builder icon.png rather than all point to the same location. The dotnet repo is still using the url, for reference.

Thanks @ryanlengel . In that case, I can merge in your PR for License. For IconUrl, is it alright to ignore NU5048 since we cannot change the file location?

Yes, I added NU5048 to NoWarn so it shouldn't warn us any more. PR merged, feel free to cherry-pick to your branch! :)

@p-nagpal
Copy link
Contributor Author

p-nagpal commented Jul 9, 2021

Abandoning this PR. Will merge 1197 from main instead.

@p-nagpal p-nagpal closed this Jul 9, 2021
@p-nagpal p-nagpal deleted the user/p-nagpal/build-fix branch July 9, 2021 15:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants