Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Brokered services #47216

Merged
merged 23 commits into from
Sep 11, 2020
Merged

Brokered services #47216

merged 23 commits into from
Sep 11, 2020

Conversation

tmat
Copy link
Member

@tmat tmat commented Aug 27, 2020 •
edited
Loading

Implements infrastructure for defining and calling IServiceBroker based services while keeping existing infrastructure until we can move all services to ISB.
Migrates some services to ISB.

Updates msbuild targets that generate ServiceHub service.json files to also generate VSIX manifest entries. Moves the list of services to a separate file Services.props.

Partially implements #40512.
Partially implements #40476 for ISB services.
Addresses #44328 for ISB services.

For reference: Internal IServiceBroker docs

ISB services:

  • RemoteDiagnosticAnalyzerService
  • SemanticClassification
  • SemanticClassificationCache
  • DesignerAttribute
  • ProjectTelemetry
  • TodoComments

@tmat tmat force-pushed the BrokeredServices branch 4 times, most recently from 6cfc992 to 35c4ea4 Compare September 2, 2020 18:05
@tmat tmat marked this pull request as ready for review September 2, 2020 22:03
@tmat tmat requested review from a team as code owners September 2, 2020 22:03
CyrusNajmabadi
CyrusNajmabadi reviewed Sep 3, 2020
View reviewed changes
src/VisualStudio/Core/Def/Implementation/TodoComments/VisualStudioTodoCommentsService.cs Outdated
cancellationToken).ConfigureAwait(false);

_lazyConnection = connection;
Copy link
Member

Choose a reason for hiding this comment

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

  1. if you're intentionally setting this at the end, state why that's important. otherwise, set on the line you create the connection.
  2. rename _lazyConnection to _connection.

Copy link
Member Author

Choose a reason for hiding this comment

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

The connection is lazy though as it is initialized on demand.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do [1]

@tmat
Copy link
Member Author

tmat commented Sep 10, 2020

/azp run Windows_Desktop_Unit_Tests release_32

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@tmat
Copy link
Member Author

tmat commented Sep 10, 2020

/azp run roslyn-CI

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tmat
Copy link
Member Author

tmat commented Sep 11, 2020

/azp run roslyn-CI (Windows_Desktop_Unit_Tests release_64)

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@tmat tmat merged commit e53ffb7 into dotnet:master Sep 11, 2020
@ghost ghost added this to the Next milestone Sep 11, 2020
@tmat tmat deleted the BrokeredServices branch September 11, 2020 18:30
@mavasani mavasani mentioned this pull request Sep 17, 2020
Closed
@dibarbet dibarbet modified the milestones: Next, 16.8.P4 Sep 22, 2020
@tmat tmat mentioned this pull request Oct 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidwengier davidwengier davidwengier left review comments

@sharwell sharwell sharwell left review comments

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
16.8.P4
Development

Successfully merging this pull request may close these issues.
5 participants
@tmat @davidwengier @sharwell @CyrusNajmabadi @dibarbet