Runtime: S.T.Json remarks section from the xml comments doesn't port well (there should be just one per API)

[ahsonkhan]

A few of the System.Text.Json APIs have multiple remarks tags within the xml comments. This does not work well when ported to the https://github.com/dotnet/dotnet-api-docs repo since only one remarks tag is allowed. We need to fix the comments in source such that there is only a single remarks tag, and then we can update the docs repo accordingly.

An example where we see an issue:
https://github.com/dotnet/corefx/blob/c0ed16860e62eea24620b7144ef72d5819ddb2b2/src/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.cs#L314-L321

Note that only one of the remarks shows up.
https://github.com/dotnet/dotnet-api-docs/blob/ebb4761136a7baba7c8a42784803af99b139f67d/xml/System.Text.Json/Utf8JsonWriter.xml#L188-L197

https://docs.microsoft.com/en-us/dotnet/api/system.text.json.utf8jsonwriter.dispose?view=netcore-3.0#remarks

An example of what the remarks should look like (single remarks tag with para tags to separate each one):
https://github.com/dotnet/corefx/blob/c0ed16860e62eea24620b7144ef72d5819ddb2b2/src/System.Text.Json/src/System/Text/Json/Document/JsonElement.cs#L124-L133

https://github.com/dotnet/dotnet-api-docs/blob/ebb4761136a7baba7c8a42784803af99b139f67d/xml/System.Text.Json/JsonElement.xml#L822-L830

https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonelement.trygetdouble?view=netcore-3.0#remarks

Anywhere we have multiple remarks tags needs to be fixed by merging it into one (this applies across all the Json APIs). Take another example:
https://github.com/dotnet/corefx/blob/c0ed16860e62eea24620b7144ef72d5819ddb2b2/src/System.Text.Json/src/System/Text/Json/Reader/Utf8JsonReader.cs#L93-L99

cc @carlossanlop, @mairaw

[Gnbrkm41]

Would like to work on it (in fact I finished it, just waiting for it to build & run tests....).

[ahsonkhan]

Great, @Gnbrkm41. Thanks for the quick fix!

We would also want to port the fix to the docs repo once it's checked in here.

Assigning to myself as a placeholder.

[ahsonkhan]

@danmosemsft, can we add @Gnbrkm41 as a collaborator so I can assign the issue to them?

[danmosemsft]

Sure, @Gnbrkm41 I just sent you an invite. When you accept, we can assign. Note this auto subscribes to the repo which is a lot of traffic, you will likely want to switch that off. Thanks for contributing! We can still take contributiosn without assigning things. It's just helpful

[Gnbrkm41]

Done. :^)

[mairaw]

I imagine @carlossanlop tool could help with the porting to docs. Otherwise, our reflection tool should be able to.

[ahsonkhan]

Since these are edits to existing docs (and are not new xml comments), I am not sure if the tool will be able to pick up the changes.

Re-opening to track fixing in the docs repo.

[Gnbrkm41]

I can fix the docs once I finish this other PR I'm preparing...... I think. Or maybe I could prioritise this first? 😄

[Gnbrkm41]

Are all the xml comments for System.Text.Json migrated to dotnet/dotnet-api-docs repo? JsonEncodedText seems relatively untouched (as in, no details are filled in). I'm not sure if I'm supposed to fill in everything manually or if the tool is able to generate the missing bits.

[ahsonkhan]

Are all the xml comments for System.Text.Json migrated to dotnet/dotnet-api-docs repo?

Not yet, not all of them have been migrated (like JsonSerializer - https://github.com/dotnet/dotnet-api-docs/blob/master/xml/System.Text.Json.Serialization/JsonSerializer.xml)

cc @steveharter

The ones that are empty will be filled in by the tool (like JsonEncodedText). It's the ones that have "merge conflicts" with existing text that won't be. These remarks issue falls in that category.

[Gnbrkm41]

Right, I'll leave the empty ones for now then.

[mairaw]

If you want to open an issue in dotnet/dotnet-api-docs, we can track the porting of the comments. Our reflection tool can override comments.

[Gnbrkm41]

I think it's better to let the tool do it's job... but I'll wait for how Ahson thinks.

[ericstj]

@ahsonkhan / @carlossanlop did these make their way into docs repo?

[ahsonkhan]

This issue has been fixed by https://github.com/dotnet/dotnet-api-docs/pull/2629 and some of the later edits made by reflecting over the JsonEncodedText/JsonSerializer APIs and auto-porting the xml comments from the source (such as https://github.com/dotnet/dotnet-api-docs/pull/2737).

I verified that all the relevant doc changes have gone through.
For example, all of these have the correct/multiple remarks:

• https://docs.microsoft.com/en-us/dotnet/api/system.text.json.utf8jsonwriter?view=netcore-3.0#remarks
• https://docs.microsoft.com/en-us/dotnet/api/system.text.json.utf8jsonwriter.disposeasync?view=netcore-3.0#remarks
• https://docs.microsoft.com/en-us/dotnet/api/system.text.json.utf8jsonwriter.writestring?view=netcore-3.0#System_Text_Json_Utf8JsonWriter_WriteString_System_Text_Json_JsonEncodedText_System_DateTime_
• https://docs.microsoft.com/en-us/dotnet/api/system.text.json.utf8jsonreader.tokenstartindex?view=netcore-3.0
• https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonencodedtext.tostring?view=netcore-3.0#System_Text_Json_JsonEncodedText_ToString
• https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializer.deserialize?view=netcore-3.0#System_Text_Json_JsonSerializer_Deserialize_System_Text_Json_Utf8JsonReader__System_Type_System_Text_Json_JsonSerializerOptions_

Thanks @Gnbrkm41 for helping with this issue!