Skip to content

Simplify SolutionEventMonitor#58868

Merged
sharwell merged 2 commits intodotnet:mainfrom
sharwell:slneventmonitor
Jan 20, 2022
Merged

Simplify SolutionEventMonitor#58868
sharwell merged 2 commits intodotnet:mainfrom
sharwell:slneventmonitor

Conversation

@sharwell
Copy link
Copy Markdown
Contributor

Closes #58827

@sharwell sharwell requested a review from a team as a code owner January 14, 2022 20:47
@ghost ghost added the Area-IDE label Jan 14, 2022
@jasonmalinowski jasonmalinowski dismissed their stale review January 14, 2022 23:17

Actually have one concern here.


private readonly UIContext? _solutionClosingContext;
private IGlobalOperationNotificationService? _notificationService;
private readonly UIContext _solutionClosingContext = UIContext.FromUIContextGuid(VSConstants.UICONTEXT.SolutionClosing_guid);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just to confirm this is safe to call on any thread at this point? I know in the past there's been some gotchas with this method since it'd implicitly call into COM APIs and I think those were resolved, but not able to directly find the answer.

Copy link
Copy Markdown
Contributor Author

@sharwell sharwell Jan 18, 2022

Choose a reason for hiding this comment

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

➡️ Since this isn't a static variable, it's effectively the same as placing it the constructor like it was previously. In the original code, it was gated behind an if check, but the service in question for that check is always present.

@sharwell sharwell merged commit 4dfc18e into dotnet:main Jan 20, 2022
@sharwell sharwell deleted the slneventmonitor branch January 20, 2022 18:34
@ghost ghost added this to the Next milestone Jan 20, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P1 Feb 4, 2022
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.

Cleanup SolutionEventMonitor type

4 participants