Skip to content

Conversation

@DustinCampbell
Copy link
Member

This change links all of the files from Microsoft.AspNetCore.Razor.Utilities.Shared into Microsoft.AspNetCore.Razor.SourceGenerator.Tooling.Internal, now that Utilities.Shared is referenced by Microsoft.AspNetCore.Razor.Language. Otherwise, SourceGenerators.Tooling.Internal will fail to build if anything from Utilities.Shared is actually used.

@333fred: I noticed that Microsoft.AspNetCore.Razor.SourceGenerator.Tooling.Internal does not directly reference System.Collections.Immutable, so it's transitively referencing it through its reference to Microsoft.CodeAnalysis.CSharp. Would it make sense to add a direct reference in SourceGenerator.Tooling.Internal to ensure that it builds with the same version as Microsoft.AspNetCore.Razor.Language?

@DustinCampbell DustinCampbell requested a review from a team as a code owner February 16, 2023 18:17
@333fred
Copy link
Member

333fred commented Feb 16, 2023

I believe this will conflict with the changes in #8245, where I added the shared lib as a project reference to MS.AspNet.Language

@DustinCampbell
Copy link
Member Author

DustinCampbell commented Feb 16, 2023

I believe this will conflict with the changes in #8245, where I added the shared lib as a project reference to MS.AspNet.Language

I wasn't sure if a project reference was legit, since there was such care taken to link everything. If your change is the correct one, I'm happy to take that over this.

@DustinCampbell
Copy link
Member Author

@dotnet/razor-compiler: I've enabled auto-merge. Feel free to add a green checkmark to let this go in before #8245.

@333fred 333fred disabled auto-merge February 17, 2023 22:39
@333fred
Copy link
Member

333fred commented Feb 17, 2023

I've disabled auto-merge, as we require 2 reviews for compiler changes. Sorry 🙂.

Copy link
Member

@chsienki chsienki left a comment

Choose a reason for hiding this comment

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

LGTM thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants