search

dotnet / docs

This repository contains .NET Documentation.
https://learn.microsoft.com/dotnet
Creative Commons Attribution 4.0 International
4.18k stars 5.84k forks source link

fix the conceptual docs mentioned in https://github.com/dotnet/dotnet-api-docs/pull/4640#issuecomment-690732459 #22857

Open apenn-msft opened 3 years ago

apenn-msft commented 3 years ago

We should close this PR and instead fix the conceptual docs mentioned in https://github.com/dotnet/dotnet-api-docs/pull/4640#issuecomment-690732459

Originally posted by @ManickaP in https://github.com/dotnet/dotnet-api-docs/issues/4640#issuecomment-778452417

Edit by @adegeo to add doc metadata

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

adegeo commented 3 years ago

@apenn-msft would you be able to summarize the work you're requesting in this issue directly?

apenn-msft commented 3 years ago

@apenn-msft would you be able to summarize the work you're requesting in this issue directly?

it's in the linked comment. in a one-line sentence "in various Asynchronous Programming Model documentation refers to "async callbacks" invoked "in another thread" however the callbacks in certain contexts may not be invoked asynchronously". (in practice, this resulted in reentrancy issues that took a bit of debugging to realize the documentation was incorrect). verbatim:

Callbacks might be invoked synchronously everywhere.

AsyncCallback is quite an unfortunate name we've chosen then. At best, it could be ambiguous whether "Async" refers to the IAsyncResult parameter or it being asynchronously executed (once we remove the part of the documentation that explicitly states the latter)

Can we link a follow-up PR/issue to fix the documentation here to not mention "in another thread": https://docs.microsoft.com/en-us/dotnet/api/system.asynccallback?view=netcore-3.1 https://docs.microsoft.com/en-us/dotnet/standard/asynchronous-programming-patterns/using-an-asynccallback-delegate-to-end-an-asynchronous-operation

there may be other instances in the surrounding APM documentation so it would be good to also conduct a brief review.

adegeo commented 3 years ago

I'll add it to the backlog. If you would like to contribute, you can submit the fix by editing the original article. Click on the Content Source link at the bottom of your first comment. To learn how to edit, see the Editing files in a repository article from GitHub.

Thanks again!

apenn-msft commented 3 years ago

@adegeo this is what I did; the edit was rejected because the updated documentation should not be specific to the original document, but needs to be updated across all documentation that references async results. There are potentially many-such articles.

adegeo commented 3 years ago

@apenn-msft ahh OK! Sorry for the confusion. We can use this issue to gather the information needed, such as which articles need to be updated.

@BillWagner Has there been some ongoing work in this area? I seem to remember something, but maybe I'm wrong.

BillWagner commented 3 years ago

Has there been some ongoing work in this area?

Not that I know of. ping @IEvangelist

IEvangelist commented 3 years ago

Hi @adegeo and @BillWagner,

It was something that we were working on about a year ago now, closer to when I joined the team - but I do not believe that it's been something that's been a recent focus. I do know that there is a project for async stuff specifically, other than that I'm not sure.

Async, await, tasks, and parallel programming in .NET