Skip to content

Use task.IsCompletedSuccessfully rather than ReferenceEquals#11606

Merged
jkotalik merged 2 commits into
dotnet:masterfrom
benaadams:IsCompletedSuccessfully
Jul 12, 2019
Merged

Use task.IsCompletedSuccessfully rather than ReferenceEquals#11606
jkotalik merged 2 commits into
dotnet:masterfrom
benaadams:IsCompletedSuccessfully

Conversation

@benaadams
Copy link
Copy Markdown
Member

As ASP.NET Core now targets .NET Core it can use task.IsCompletedSuccessfully; this is preferable to comparing to Task.CompletedTask as an async method uses a different completed task than Task.CompletedTask so it will catch more instances.

Also will make @stephentoub happier 😄

@stephentoub
Copy link
Copy Markdown
Member

so it will catch more instances

Including asynchronously completing operations that then complete fast enough that they're complete by the time the check happens.

@davidfowl
Copy link
Copy Markdown
Member

Much better,

@analogrelay analogrelay added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-servers labels Jun 26, 2019
Copy link
Copy Markdown
Member

@rynowak rynowak left a comment

Choose a reason for hiding this comment

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

<3

Comment thread src/Mvc/Mvc.ViewFeatures/src/Buffers/PagedBufferedTextWriter.cs Outdated
@analogrelay analogrelay added this to the 3.0.0-preview7 milestone Jun 27, 2019
@mkArtakMSFT mkArtakMSFT removed this from the 3.0.0-preview7 milestone Jul 10, 2019
@benaadams benaadams force-pushed the IsCompletedSuccessfully branch from efce6af to c89352b Compare July 12, 2019 18:10
@benaadams
Copy link
Copy Markdown
Member Author

Rebased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants