Skip to content

Conversation

@333fred
Copy link
Member

@333fred 333fred commented Jun 1, 2023

No description provided.

@333fred 333fred force-pushed the add-to-slnf branch 2 times, most recently from da9fea2 to a88923d Compare June 27, 2023 00:54
@333fred 333fred marked this pull request as ready for review June 27, 2023 17:59
@333fred 333fred requested review from a team as code owners June 27, 2023 17:59
@333fred 333fred requested a review from davidwengier June 27, 2023 17:59
@333fred
Copy link
Member Author

333fred commented Jun 27, 2023

@dotnet/razor-compiler for review. I would suggest commit-by-commit review for ease of understanding the changes.

@dotnet/razor-tooling f43572a is the one commit that you really need to review. There isn't a shared test library between compiler and tooling, so I didn't add the testing utility reference to the tooling layer, but you should feel free to add it and start taking advantage of the diff helpers if you want.

#endregion

private class RazorCapabilitiesProvider : IRazorCapabilitiesProvider
private class RazorCapabilitiesProvider : IRazorTestCapabilitiesProvider
Copy link
Member Author

Choose a reason for hiding this comment

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

IRazorCapabilitiesProvider was marked obsolete.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, noticed this change merged in last night, and was going to respond today :)

Copy link
Member

@davidwengier davidwengier Jun 27, 2023

Choose a reason for hiding this comment

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

Turns out this breaks running tooling tests locall, so going to have to revert. We need to wait for these roslyn bits to be in a VS build.

EDIT: To be clear, I'll just revert this specific change to the RazorCapabilitiesProvider. The rest of the PR is fine :D

Copy link
Member

Choose a reason for hiding this comment

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

Nope, even reverting this, just having the new roslyn dep breaks running unit tests for me. I don't know what to think any more.

@333fred 333fred merged commit 450c0a1 into dotnet:main Jun 27, 2023
@333fred 333fred deleted the add-to-slnf branch June 27, 2023 20:41
@333fred
Copy link
Member Author

333fred commented Jun 27, 2023

Thanks everyone!

@davidwengier
Copy link
Member

you should feel free to add it and start taking advantage of the diff helpers if you want.

We currently have an uglier way to acheive the same result:

I will try to make them nicer, but annoyingly, adding the reference to the CodeAnalysis.Testing package introduces some namespace collisions.

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.

5 participants