-
Notifications
You must be signed in to change notification settings - Fork 378
[9.0] Generate proper Visual Studio component IDs #15103
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
Conversation
| safeId = safeId.StartsWith("Microsoft.NET.", StringComparison.OrdinalIgnoreCase) ? safeId.Substring(14) : | ||
| safeId.StartsWith("Microsoft.", StringComparison.OrdinalIgnoreCase) ? safeId.Substring(10) : |
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'd prefer to see this as a loop over the well-known prefixes, without hard-coding the length of each one (ie the 10 and 14 constants).
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.
Fixed, thank you.
| i => i.ItemSpec.Contains("Microsoft.NET.Component.sdk.emscripten.5.6.swixproj")).ItemSpec), "component.swr")); | ||
| Assert.Contains("package name=Microsoft.NET.Component.sdk.emscripten", componentSwr); |
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.
Does this casing make sense? The first half uses proper upper case and the second half uses lowercase only. Is that how VS does it too?
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 workload IDs are case-sensitive - And while we can probably fix this for some, others might prove hard, e.g. maccatalyst. If we followed the VS model 100%, we'd probably want to end up with MacCatalyst.
| // Component prefix is defaults to null when creating a SWIX component, so these values remain | ||
| // unchanged. |
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: Just a random question. Is the split in the comment line something done automatically or something you manually pressed "Enter" and then started a new line?
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.
That's me
Description
Visual Studio publishes all the component and component group IDs from their catalog to support extension developers. The norm is to follow their naming convention and include a prefix like
Microsoft.VisualStudio.Componentin the ID. .NET optional workloads (including abstract workloads) generate components and component groups that matches the .NET workload ID. This enables features like IPA to easily locate components when SDK resolvers report missing optional workloads.Visual Studio asked that we follow their guidelines so starting in .NET 9, we will prefix our components with 'Microsoft.NET.Component'. Repos will be able to opt-in and generate new IDs.
Risk
Medium - Runtime, EMSDK, SDK, Maui all need to use the new feature simultaneously and we won't discover all the issues until we can insert new workloads and do full E2E testing. Risk is mitigated by having this as an opt-in feature.
Testing
We have sufficient unit tests to feel confident about the change. We'll be able to further assess problems once the first repos onboard to it and again when we have full E2E testing.