-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Adding ClassLibrary (.NET Core) and Creating new VSIX for ClassLibrary (.NET Standard) #510
Conversation
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.
LGTM, though I can't really speak for the template setup. Also I realized that you marked this as a WIP only after I'd already looked at it, so I'm not sure if it was ready for review or not. :-)
<Metadata> | ||
<Identity Id="45474e92-a5d6-4ffb-8689-d7e8064cec90" Version="|%CurrentProject%;GetBuildVersion|" Language="en-US" Publisher="Microsoft" /> | ||
<DisplayName>CSharp Standard Templates</DisplayName> | ||
<Description xml:space="preserve">CSharp Standard Templates</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.
The DisplayName and Description should probably be "C# .NET Standard Templates".
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.
Yes. Can we also make the project name\folder name be CSharpNetStandardTemplatesSetup?
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.
Do the display name for the template vsixes surface to the user?
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.
Normally yes (in the extension manager dialog). However since these VSIXes will be marked as Productcomponent they won't show up in the package manager dialog. So no.
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.
Good because otherwise we have missed loc strings ;)
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 did add a new string in roslyn project system dotnet/project-system#965
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.
Yes thanks. This is a different string that doesn't surface if I've understood correctly
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.
We also need authoring for the new vsix
<Metadata> | ||
<Identity Id="45474e92-a5d6-4ffb-8689-d7e8064cec90" Version="|%CurrentProject%;GetBuildVersion|" Language="en-US" Publisher="Microsoft" /> | ||
<DisplayName>CSharp Standard Templates</DisplayName> | ||
<Description xml:space="preserve">CSharp Standard Templates</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.
Yes. Can we also make the project name\folder name be CSharpNetStandardTemplatesSetup?
<ProjectTypeGuids>{82b43b9b-a64c-4715-b499-d71e9ca2bd60};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids> | ||
<ProjectGuid>{440F1724-1830-4C7A-9500-8D9323A7FE71}</ProjectGuid> | ||
<OutputType>Library</OutputType> | ||
<AssemblyName>CSharpStandardClassLibrary</AssemblyName> |
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.
Call these projects CSharpNetStandardClassLibrary as well?
eb024b2
to
7aaffdf
Compare
7aaffdf
to
8e90665
Compare
Adding Authoring for NetStandard Templates
8e90665
to
bfc2084
Compare
Thanks @dsplaisted this should be now ready for review :-) |
|
||
<ItemGroup> | ||
<MergeManifest Include="$(OutputPath)\Microsoft.VisualStudio.Templates.CSharp.NetStandard.json" /> | ||
<MergeManifest Include="$(OutputPath)\Microsoft.NET.Sdk.json" /> |
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 shouldn't include the NET SDK - otherwise the same component is being included in two vsmanprojs. Ideally we should split out the SDK into a separate vsmanproj later when we move to the model for pulling in unzipped sdks.
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 modulo the one comment about not including the SDK in the new component.
…310.1 (dotnet#510) This change updates the following dependencies - Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19160.1
fixes #441
@srivatsn @dsplaisted