-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Make System.Formats.Asn1 triple slash documentation the source of truth #108982
Conversation
Tagging subscribers to this area: @dotnet/area-system-formats-asn1, @bartonjs, @vcsjones |
cc @carlossanlop in case I missed anything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this!
@@ -41,15 +41,15 @@ public static partial class AsnDecoder | |||
/// <paramref name="ruleSet"/> is not defined. | |||
/// </exception> | |||
/// <exception cref="AsnContentException"> | |||
/// the next value does not have the correct tag. | |||
/// The next value does not have the correct tag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These might need <para>
tags so they remain on separate lines in the docs, right @carlossanlop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now that that would mean a lot of changes throughout this PR. I'm also fine with not adding them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The porttotripleslash tool did not put <para> there, so I assumed it was not required.
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnDecoder.BitString.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnDecoder.BitString.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnDecoder.Enumerated.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnDecoder.NamedBitList.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnDecoder.SetOf.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnWriter.OctetString.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnWriter.OctetString.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnWriter.SetOf.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnWriter.SetOf.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (modulo @gewarren's suggestions). Can you please backport it to release/9.0 when you're done?
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/Asn1Tag.Accelerators.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnDecoder.NamedBitList.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnDecoder.NamedBitList.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnDecoder.Oid.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Asn1/src/System/Formats/Asn1/AsnWriter.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Genevieve Warren <[email protected]> Co-authored-by: Jeremy Barton <[email protected]>
Co-authored-by: Jeremy Barton <[email protected]>
Co-authored-by: Genevieve Warren <[email protected]>
@carlossanlop Is there a documented set of steps needed to set this repo as the source of truth? If so, can you add a step to disallow edits in the dotnet-api-docs repo for the affected namespace? Like this. |
Correct 👍 and big thanks for doing this. |
/backport to release/9.0 |
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/11392049693 |
The pull request makes the triple slash documentation for System.Formats.Asn1 the source of truth for documentation.
Use the
PortToTripleSlash
tool to update the C# documentation with the latest documentation from dotnet-api-docs.This made substantial changes that were largely formatting, which I reverted. I kept the changes to two things: actual content changes (text changes, missing documentation, etc) and re-ordering XML elements as appropriate (param elements not matching parameter order, putting remarks at the end)
Remove
<UseCompilerGeneratedDocXmlFile>false</UseCompilerGeneratedDocXmlFile>
. The default value for this is true. I can't remember where, but I recall that our preference is to just remove it entirely, not explicitly set it to true.