-
Notifications
You must be signed in to change notification settings - Fork 692
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
Improve the license UI and messaging. #2507
Conversation
Nit: “View License” and not “ViewLicense” |
@anangaur It is "View License" in the code, I just missed the space in the description :) |
@@ -140,7 +141,7 @@ | |||
<comment>0 - the license expression, 1 further details</comment> | |||
</data> | |||
<data name="InvalidLicenseExpressionVersion" xml:space="preserve"> | |||
<value>The version string '{0}' for the License Expression is invalid.</value> | |||
<value>The version string '{0}' PackageLicenseExpressionVersion is invalid.</value> |
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.
PackageLicenseExpressionVersion '{0}' version string is invalid.
e8603d0
to
c7544f6
Compare
c7544f6
to
626ccac
Compare
@@ -130,7 +130,8 @@ | |||
<comment>{0} is the file being packed.</comment> | |||
</data> | |||
<data name="InvalidLicenseCombination" xml:space="preserve"> | |||
<value>Invalid metadata. Cannot specify both a License Expression and a License File.</value> | |||
<value>Invalid metadata. Cannot specify both a PackageLicenseExpression and a PackageLicenseFile.</value> | |||
<comment>Please don't localize PackageLicenseUrl, PackageLicenseFile and PackageLicenseExpression.</comment> |
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.
nit: PackageLicenseUrl
is not used in this string, remove it from the comment.
<data name="DefaultError_NoMatchInAllowList" xml:space="preserve"> | ||
<value>The package signature certificate fingerprint does not match any certificate fingerprint in the allow list.</value> | ||
<data name="NuGetLicenseExpression_NonStandardIdentifier" xml:space="preserve"> | ||
<value>The license identifier(s) {0} is(are) not recognized by the current toolset.</value> |
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.
nit: '{0}'
uGet.Core/NuGet.Build.Tasks.Pack/Strings.resx
Bug
Fixes: NuGet/Home#7462, part of NuGet/Home#7467
Regression: No
If Regression then when did it last work:
If Regression then how are we preventing it in future:
Fix
Details:
Some strings changes based on the feedback from Karan.
Earlier in 15.8 in the details pane for the license field we showed a clickable license url. Per Karan's feedback, changing that to View License, which is the equivalent of what we do for embedded files.
The benefit is that an ugly url is not shown there. The con is that you don't know the exact link.
From NuGet/Home#7467 the following are fixed.
The whitespace issues with license expressions are fixed.
The weight of the header is the same as the text on the left in the same row.
The license file window appears at the middle of the screen.
The hyperlinks display a tooltip.
//cc @karann-msft @rrelyea @anangaur
Testing/Validation
Tests Added: No
Reason for not adding tests: Strings changes.
Validation done: