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

Remove ForegroundThreadAffinitizedObject usage #60526

Merged
merged 35 commits into from
Apr 2, 2022
Merged

Remove ForegroundThreadAffinitizedObject usage #60526

merged 35 commits into from
Apr 2, 2022

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 1, 2022 16:56
@genlu
Copy link
Member

genlu commented Apr 1, 2022

Why do we prefer using ThreadingContext directly over ForegroundThreadAffinitizedObject?

@Cosifne
Copy link
Member

Cosifne commented Apr 1, 2022

Why do we prefer using ThreadingContext directly over ForegroundThreadAffinitizedObject?

Same question here : )

@jasonmalinowski
Copy link
Member

Ditto on the question -- if we want to get rid of ForegroundThreadAffinitizedObject I won't mind too much, but if that's the direction we want to go we should excise it everywhere.

@CyrusNajmabadi
Copy link
Member Author

Ditto on the question -- if we want to get rid of ForegroundThreadAffinitizedObject I won't mind too much, but if that's the direction we want to go we should excise it everywhere.

Workingon it.

@CyrusNajmabadi
Copy link
Member Author

Why do we prefer using ThreadingContext directly over ForegroundThreadAffinitizedObject?

So FTAO is a legacy thing we built prior to even having JTF and the nice APIs it has. Today it's just a thin wrapper around IThreadingContext and really serves no good purpose. So i'd prefer we just consistently use the platform compoennt here.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 1, 2022 21:29
@CyrusNajmabadi CyrusNajmabadi requested a review from sharwell April 1, 2022 21:40
@CyrusNajmabadi CyrusNajmabadi enabled auto-merge April 1, 2022 21:41
sharwell
sharwell approved these changes Apr 1, 2022
View reviewed changes
src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager.cs
{
public readonly IThreadingContext ThreadingContext;
Copy link
Member

Choose a reason for hiding this comment

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

Why public?

src/EditorFeatures/Core/GoToDefinition/AbstractGoToCommandHandler`2.cs Outdated
@@ -94,7 +94,7 @@ public CommandState GetCommandState(TCommandArgs args)
public bool ExecuteCommand(TCommandArgs args, CommandExecutionContext context)
{
// Should only be called on the UI thread.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Should only be called on the UI thread.

src/EditorFeatures/Core/GoToDefinition/AbstractGoToCommandHandler`2.cs Outdated
@@ -129,7 +129,7 @@ public bool ExecuteCommand(TCommandArgs args, CommandExecutionContext context)
try
{
// Should only be called on the UI thread.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Should only be called on the UI thread.

@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
08d1cf6
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
81ff6f7
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
b93ec77
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
bd29c17
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
c95a831
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
cb6aab6
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
c29f412
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
fbe4f0b
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
aa4aa04
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
59e809a
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
75770d7
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
999f56f
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
20fe63b
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
664a2d4
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
1ba8010
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
23b0791
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
8a6dc3e
@CyrusNajmabadi
Remove ForegroundThreadAffinitizedObject usage
27d9a8d
@CyrusNajmabadi CyrusNajmabadi merged commit ce61483 into dotnet:main-vs-deps Apr 2, 2022
@ghost ghost added this to the Next milestone Apr 2, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the removeFTAO branch April 3, 2022 17:41
@dibarbet dibarbet mentioned this pull request Apr 6, 2022
Closed
@dibarbet dibarbet modified the milestones: Next, 17.3.P1 Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sharwell sharwell sharwell approved these changes

@genlu genlu genlu approved these changes

Assignees
No one assigned
Labels
Area-IDE
Projects
None yet
Milestone
17.3.P1
Development

Successfully merging this pull request may close these issues.
6 participants
@CyrusNajmabadi @genlu @Cosifne @jasonmalinowski @sharwell @dibarbet