Skip to content

UnitTesting and Razor External Access wrappers for ISB services#48776

Merged
sharwell merged 5 commits intodotnet:masterfrom
tmat:CallbackDispatchers3
Nov 5, 2020
Merged

UnitTesting and Razor External Access wrappers for ISB services#48776
sharwell merged 5 commits intodotnet:masterfrom
tmat:CallbackDispatchers3

Conversation

@tmat
Copy link
Member

@tmat tmat commented Oct 19, 2020

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@tmat tmat force-pushed the CallbackDispatchers3 branch from f5c8355 to f9a5d05 Compare October 23, 2020 21:49
@tmat tmat changed the title Callback dispatchers3 UnitTesting External Access wrappers for ISB services Oct 23, 2020
@tmat tmat marked this pull request as ready for review October 23, 2020 23:01
@tmat tmat requested review from a team as code owners October 23, 2020 23:01
@tmat tmat added the Area-IDE label Oct 24, 2020
@tmat
Copy link
Member Author

tmat commented Oct 24, 2020

@shyamnamboodiripad PTAL

@tmat
Copy link
Member Author

tmat commented Oct 24, 2020

@dotnet/roslyn-ide

@tmat tmat force-pushed the CallbackDispatchers3 branch from 72feefd to 6fdcc85 Compare October 26, 2020 20:00
@tmat tmat changed the title UnitTesting External Access wrappers for ISB services UnitTesting and Razor External Access wrappers for ISB services Oct 30, 2020
@tmat tmat force-pushed the CallbackDispatchers3 branch from b9556e4 to 9495b60 Compare October 30, 2020 23:38
{
internal interface IRemoteServiceCallbackDispatcherProvider
{
IRemoteServiceCallbackDispatcher GetDispatcher(Type serviceType);
Copy link
Contributor

Choose a reason for hiding this comment

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

😕 This seems like something the partner would manage as a completely independent brokered service. The only part they need from us is a brokered service that gives access to a specific solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could remove these but then everyone would need to reimplement pretty much the same logic we have in Roslyn. I opted to make that infrastructure available, so it's easy to use ISB for partners.

Copy link
Member Author

Choose a reason for hiding this comment

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

We also don't want to expose the different descriptors for 32/64/64S.

Copy link
Contributor

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

Overall looks fine. Eventually I'd like to see the IVT dependencies here reduced, first by eliminating the use of Roslyn's callback dispatchers (these can be implemented by each partner separately, and if it's too complex then ServiceHub itself needs to provide this functionality), and then by moving the services to independent brokered services that depend only on Roslyn's brokered service that provides access to the workspace and/or solution.

We need to make sure this works with Server GC scenarios as well.

@sharwell sharwell merged commit 7e5590c into dotnet:master Nov 5, 2020
@ghost ghost added this to the Next milestone Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move LUT and SBD services to ISB

5 participants