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.

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

Remove TODO from public documentation #47370

Merged
merged 3 commits into from
Sep 5, 2020

Conversation

Youssef1313
Copy link
Member

Fixes #47366

There are more TODOs in triple-slash doc comments. But I only found this one in a public API.

@Youssef1313 Youssef1313 requested a review from a team as a code owner September 2, 2020 12:48
Copy link
Member

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

Rather than remove the semantic reference (the <see> element), you can change <remarks> to <devdoc> which continues to be validated by the compiler but doesn't show in rendered documentation.

@jcouv jcouv added Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. Documentation labels Sep 2, 2020
@Youssef1313
Copy link
Member Author

Rather than remove the semantic reference (the <see> element), you can change <remarks> to <devdoc> which continues to be validated by the compiler but doesn't show in rendered documentation.

Thanks. I didn't know about the existence of devdoc.

@sharwell
Copy link
Member

sharwell commented Sep 2, 2020

I didn't know about the existence of devdoc.

It's a made-up XML element historically used for this purpose. The documentation compiler defines several well-known elements, but tools are allowed to define and/or use others.

Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>
@Youssef1313
Copy link
Member Author

Youssef1313 commented Sep 2, 2020

@sharwell I found that a devdoc was used in dotnet/runtime but was included in the public documentation:

https://github.com/dotnet/runtime/blob/6072e4d3a7a2a1493f514cdf4be75a3d56580e84/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ThreadWaitReason.cs#L6-L9

https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.threadwaitreason?view=netcore-3.1

The XML for the doc page include the exact same text as summary.

Any idea why this is happening? Was that a manual change to the generated XML?

@sharwell
Copy link
Member

sharwell commented Sep 2, 2020

Here's the XML documentation for ThreadWaitReason:
https://github.com/dotnet/dotnet-api-docs/blob/78344cc2feabc5795757fe8770e7692c4f9d4211/xml/System.Diagnostics/ThreadWaitReason.xml#L80

The <devdoc> in dotnet/runtime is just for local reference but isn't the actual API documentation for this type.

@jcouv jcouv self-assigned this Sep 2, 2020
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 3)

@Youssef1313 Youssef1313 marked this pull request as draft September 4, 2020 21:40
@Youssef1313 Youssef1313 marked this pull request as ready for review September 4, 2020 21:40
@jcouv jcouv merged commit 7268be1 into dotnet:master Sep 5, 2020
@ghost ghost added this to the Next milestone Sep 5, 2020
@jcouv
Copy link
Member

jcouv commented Sep 5, 2020

Squashed/merged. Thanks @Youssef1313 !

@dibarbet dibarbet modified the milestones: Next, 16.8.P4 Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal code comments are put in triple-slash doc comments
5 participants