-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Package descriptions for CSharp and VB should include summary blurb #22166
Conversation
ubuntu_14_debug_prtest failed with CURL error below (details).
|
test ubuntu_14_debug_prtest please |
ubuntu_14_debug_prtest failed again (details) @dotnet/roslyn-infrastructure Is there a disc space issue on ubuntu machines?
|
test ubuntu_14_debug_prtest please |
test windows_determinism_prtest please |
test windows_debug_vs-integration_prtest please |
@jaredpar @dotnet/roslyn-infrastructure for review. This is fixing the description blurb for packages. Thanks |
CC @sharwell, @jasonmalinowski. Thought this appeared in the one line summary locations |
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.
Indeed, summaries are shown in the NuGet package manager UI in Visual Studio, if not other places. Do not remove it.
windows_debug_unit64_prtest failed with error below (details).
|
windows_determinism_prtest failed with a similar error (details)
|
test windows_debug_unit64_prtest please |
test windows_determinism_prtest please |
windows_debug_unit64_prtest failed again with a locked file (details). @dotnet/roslyn-infrastructure Is this a known issue?
|
My impression is @jaredpar and @nguerrera are investigating this right now. (Email also just sent internally.) |
@jasonmalinowski I added the summary blurbs back. Thanks for the feedback. Good to go? |
@@ -2,10 +2,13 @@ | |||
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd"> | |||
<metadata minClientVersion="3.3"> | |||
<id>Microsoft.CodeAnalysis.CSharp.CodeStyle</id> | |||
<summary> |
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.
Think we still need a summary here
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.
You may be looking at an older iteration. Latest commit is 4.
In reply to: 139810920 [](ancestors = 139810920)
@@ -2,10 +2,13 @@ | |||
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd"> | |||
<metadata minClientVersion="3.3"> | |||
<id>Microsoft.CodeAnalysis.CSharp.Features</id> | |||
<summary> | |||
<description> |
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.
same
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.
Sorry a bunch of nitpicks on descriptions only because you chose to touch this. 😄 If you want to punt a few of these that's fine.
Contains the build task and targets used by MSBuild to run the C# and VB compilers. | ||
Supports using VBCSCompiler on Windows. | ||
The build task and targets used by MSBuild to run the C# and VB compilers. | ||
Supports using VBCSCompiler on Windows. |
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.
I must admit I'm not sure how a customer will understand what VBCSCompiler is, much less how this package supports it on Windows.
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.
I don't know either. I'll leave this one as-is.
@@ -2,9 +2,13 @@ | |||
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd"> | |||
<metadata minClientVersion="3.3"> | |||
<id>Microsoft.CodeAnalysis.Compilers</id> | |||
<summary>.NET Compiler Platform ("Roslyn")</summary> | |||
<summary> | |||
.NET Compiler Platform ("Roslyn"). |
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.
Should this say it's the compiler layer? This otherwise sounds like it's the whole thing, which it's not.
.NET Compiler Platform ("Roslyn"). Install this package to get both C# and Visual Basic support. Install either of the dependencies directly to get one of the languages separately. | ||
.NET Compiler Platform ("Roslyn"). | ||
Install this package to get both C# and Visual Basic support. | ||
Install either of the dependencies directly to get one of the languages separately. |
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.
This sentence is entirely correct and entirely impossible to read. "If you want just support for one language, install Microsoft.CodeAnalysis.CSharp or Microsoft.CodeAnalysis.VisualBasic".
@@ -6,9 +6,13 @@ | |||
.NET Compiler Platform ("Roslyn") support for editor features inside the Visual Studio editor.. | |||
</summary> | |||
<description> | |||
.NET Compiler Platform ("Roslyn") support for editor features inside the Visual Studio editor.. |
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.
Double period. Yes, you're just copying, but still...
</summary> | ||
<description>$commitPathMessage$</description> | ||
<description> | ||
.Net Compilers package. |
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.
.NET is capitalized.
@@ -6,9 +6,13 @@ | |||
.NET Compiler Platform ("Roslyn") support for Visual Studio "15". | |||
</summary> | |||
<description> | |||
.NET Compiler Platform ("Roslyn") support for Visual Studio "15". |
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.
Uh, what should this one say? @heejaechang or @Pilchie?
* dotnet/master: (351 commits) Avoid scheduling an unnecessary dispose task for EmptyAsyncToken Support for ref-readonly locals (dotnet#22269) ported tests from dotnet#21263 (dotnet#22296) NormalizeWhitespace should not add a space in nullable type (dotnet#22234) Fix the work item number associated with the unit test Add unit test Terminate a sentence Add instructions for upgrading PowerShell Unsafe local should require /unsafe flag on compilation (dotnet#22268) CR feedback Not poisoning restricted types known to be stack only. Typo Error messsage should mention C# 7.0 not C# 7 (dotnet#22255) Package descriptions for CSharp and VB should include summary blurb (dotnet#22166) Fix a trivial issue in how IsLifted is calculated for C# compound assignment operator Do not zero out "With" statement target expression locals referenced within a lambda. (dotnet#22223) Fix unit test failure from merge Mark "private protected", "ref readonly" and other C# 7.2 features as merged (dotnet#22209) Add unit tests for IAwaitExpression and make the API public again more test fixes due to merge conflicts ...
* dotnet/master: (309 commits) Avoid scheduling an unnecessary dispose task for EmptyAsyncToken Support for ref-readonly locals (dotnet#22269) ported tests from dotnet#21263 (dotnet#22296) NormalizeWhitespace should not add a space in nullable type (dotnet#22234) Fix the work item number associated with the unit test Add unit test Terminate a sentence Add instructions for upgrading PowerShell Unsafe local should require /unsafe flag on compilation (dotnet#22268) CR feedback Not poisoning restricted types known to be stack only. Typo Error messsage should mention C# 7.0 not C# 7 (dotnet#22255) Package descriptions for CSharp and VB should include summary blurb (dotnet#22166) Fix a trivial issue in how IsLifted is calculated for C# compound assignment operator Do not zero out "With" statement target expression locals referenced within a lambda. (dotnet#22223) Fix unit test failure from merge Mark "private protected", "ref readonly" and other C# 7.2 features as merged (dotnet#22209) Add unit tests for IAwaitExpression and make the API public again more test fixes due to merge conflicts ...
Customer scenario
Find the compiler packages to add to your project on nuget.org or myget.org (for example CodeAnalysis.CSharp). The package
description
should give you a summary and a link to our package documentation (including versioning).But currently, many packages only show a commit link.
Also, I'm removing the
summary
section since it does not seem to appear anywhere.Correct:
![image](https://user-images.githubusercontent.com/12466233/30550241-b3bfe27e-9c4b-11e7-9ea7-8765f2af701e.png)
Incorrect:
![image](https://user-images.githubusercontent.com/12466233/30550259-c0fc860e-9c4b-11e7-9cc2-d524488b9b73.png)
Workarounds, if any
This does not block usage of the packages, but makes it very difficult to figure out what changes are included in a specific version.
Risk
Low (doc-only).
Is this a regression from a previous update?
Root cause analysis:
Yes, this change split the description of nuget packages into "summary" and "description", but it looks like nuget.org only displays the description in such case.
How was the bug found?
I did a manual check while investigating another package documentation issue (#21574).
@jaredpar @dotnet/roslyn-infrastructure for review. Thanks