Remove #pragma suppressions from EditorFeatures.Cocoa#56974
Remove #pragma suppressions from EditorFeatures.Cocoa#56974davidwengier merged 1 commit intodotnet:mainfrom
Conversation
|
Ping @dotnet/roslyn-ide |
| { | ||
| await ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); | ||
|
|
||
| #pragma warning disable CA2007 // Consider calling ConfigureAwait on the awaited task (containing method uses JTF) |
There was a problem hiding this comment.
This was an intentional suppression. CA2007 does not make sense in the current context, and the standard practice for this context is to implicitly use ConfigureAwait(true).
| else | ||
| { | ||
| textView = _diffViewer.InlineView; | ||
| #pragma warning disable CA2007 // Consider calling ConfigureAwait on the awaited task (containing method uses JTF) |
There was a problem hiding this comment.
I would prefer the changes in this file be reverted. I like the idea that we should be visually penalized for failing to follow a standard practice designed to maximize readability.
The other files seem fine.
There was a problem hiding this comment.
I understand what you're saying Sam, but I checked and .ConfigureAwait(true) is used in Roslyn 209 times, compared to suppression only 19 times, so I'm keeping with what seems standard in this repo. I don't agree that making things worse because they can't be perfect is the right way to go, but I don't feel strongly about this case and wouldn't block if you wanted to remove all of the .ConfigureAwait(true) calls in the repo and implement a new standard.
Mainly I just don't want to run the integration test gauntlet because I'm not having much luck with them at the moment :)
jasonmalinowski
left a comment
There was a problem hiding this comment.
Assuming the VS for Mac folks are OK with it, 
.
Fixes #48871
Just a few random ups that hadn't been cleaned.