-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Port inheritdoc tag for runtime APIs that are missing docs #8335
Conversation
@dotnet/area-extensions-configuration @dotnet/area-extensions-dependencyinjection @dotnet/area-extensions-logging @dotnet/area-extensions-primitives @dotnet/area-system-componentmodel @dotnet/ncl @dotnet/area-system-numerics @dotnet/interop-contrib @dotnet/area-system-security @dotnet/area-system-runtime @dotnet/area-system-globalization @tarekgh @jozkee |
This comment was marked as outdated.
This comment was marked as outdated.
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.
Once the remaining conversations are resolved, this LGTM.
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.
Only one more thing to address: the warnings and suggestions. Most, if not all, showed up due to the unexpected escaping of characters.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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.
Trying some fixes for the CI suggestions.
This comment was marked as outdated.
This comment was marked as outdated.
Docs Build status updates of commit 95da900: ✅ Validation status: passedThis comment lists only the first 25 files in the pull request. xml/Microsoft.Extensions.Primitives/StringValues.xml
xml/System.Numerics/BigInteger.xml
xml/System.Numerics/Complex.xml
xml/System.Security.Cryptography.Cose/CoseHeaderMap.xml
xml/System/DateTime.xml
xml/System/DateTimeOffset.xml
xml/System/Decimal.xml
This comment lists only the first 25 errors (including error/warning/suggestion) in the pull request. Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report. For any questions, please:
|
@gewarren I have high suspicion that the suggestions are mdoc bugs. I'll merge this PR but we can discuss the error messages separately in case we need to act on them. I mainly need the undoc APIs report to show these as documented. |
MS Docs is smart enough to automatically show inherited documentation if the
<inheritdoc/>
tag is present. If an API contains such tag in the intellisense xml, we can port that tag instead of copying the parent's documentation directly. But if the API contains documentation (partial or total) in the intellisense xml, we want to port that, because we should assume the owner wants to override some or all of the parent's documentation.The docs porting tool is now able to port documentation following that logic.
The undoc APIs report we generate from MS Docs is also able to assume an API to be documented if it has the inheritdoc tag, and the parent is documented.
This PR ports the inheritdoc tag only for APIs that are partially or fully undocumented, so that they stop showing in the report.
I submitted another PR #8336 that includes the rest of the inheritdoc tags that were detected in APIs that have already been fully documented before (by copying the base type or the interface's documentation).