-
Notifications
You must be signed in to change notification settings - Fork 25.2k
OpenAPI comments /3 #35082
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
OpenAPI comments /3 #35082
Conversation
|
||
The generator emits code that contains: | ||
|
||
* A cache of XML comments mapped to member identifiers: |
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 don't think we need to provide an example of the generated code here since it's an implementation detail. Just stating that a cache is used is sufficient, IMO.
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.
This is looking really good but there are a few more changes I think are needed.
|
||
### Adding XML documentation sources | ||
|
||
The [`Microsoft.AspNetCore.OpenApi`](https://www.nuget.org/packages/Microsoft.AspNetCore.OpenApi) NuGet package automatically resolves XML documentation comments from: |
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.
We refer to the thing that performs XML documentation comment processing in several different ways in this article, and I think we could avoid confusion if we settled on just one way. My preference would be "ASP.NET Core", as used in the third paragraph (that I suggested be moved to the second paragraph), rather than "OpenAPI XML documentation processing" (in the first paragraph) or "The Microsoft.AspNetCore.OpenApi
NuGet package`", as in this paragraph.
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.
Can you take a quick look at my changing from "OpenAPI XML documentation processing"?
I made this change in the doc except to the title because it seems like it needs OpenAPI XML in the title. Please suggest better wording.
### Supported XML documentation tags | ||
|
||
* [`<summary>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#summary) | ||
* [`<remarks>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#remarks) | ||
* [`<param>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#param) | ||
* [`<returns>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#returns) | ||
* [`<response>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#response) | ||
* [`<example>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#example) | ||
* [`<deprecated>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#deprecated) | ||
* [`<inheritdoc>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#inheritdoc) | ||
|
||
### Document HTTP responses | ||
|
||
Document HTTP responses with the `<response>` tag and the `code` attribute: | ||
|
||
:::code language="csharp" source="~/fundamentals/openapi/samples/10.x/aspnet-openapi-xml/api/ProjectBoardApis.cs" id="snippet_1" highlight="6-7"::: | ||
|
||
<!--```csharp | ||
/// <response code="200">Success response with data</response> | ||
/// <response code="404">Resource not found</response> | ||
```--> | ||
|
||
### Add examples | ||
|
||
To add examples to documentation, use the [`<example>`](/dotnet/csharp/language-reference/xmldoc/recommended-tags#example) tag for types or the `example` attribute for parameters: | ||
|
||
```csharp | ||
/// <example>{"name":"Sample","value":42}</example> | ||
/// <param name="id" example="42">The unique identifier</param> | ||
``` |
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 think these sections should be moved up, out of the section on the Source Generator implementation. I think a good spot would be just before the section "Customizing XML Documentation Behavior"
Co-authored-by: Mike Kistler <[email protected]>
<Target Name="DisableCompileTimeOpenApiXmlGenerator" BeforeTargets="CoreCompile"> | ||
<ItemGroup> | ||
<Analyzer Remove="$(PkgMicrosoft_AspNetCore_OpenApi)/analyzers/dotnet/cs/Microsoft.AspNetCore.OpenApi.SourceGenerators.dll" /> |
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.
using Remove
in the project file, that is,
<Analyzer Remove="$(PkgMicrosoft_AspNetCore_OpenApi)/analyzers/dotnet/cs/Microsoft.AspNetCore.OpenApi.SourceGenerators.dll" />
DOES NOT prevent \aspnet-openapi-xml\api\obj\Debug\net10.0\Api.xml
from being generated.
|
||
### Adding XML documentation sources | ||
|
||
The [`Microsoft.AspNetCore.OpenApi`](https://www.nuget.org/packages/Microsoft.AspNetCore.OpenApi) NuGet package automatically resolves XML documentation comments from: |
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.
Can you take a quick look at my changing from "OpenAPI XML documentation processing"?
I made this change in the doc except to the title because it seems like it needs OpenAPI XML in the title. Please suggest better wording.
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.
This looks great! 👍
Fixes #34980
Ask
@martincostello
for a review@captainsafia CoPilot added a bunch of text. Please suggest better text. Sign off on each
<!-- review -->
comment.I split up all the long sentences using bullets and may have changed the intended meaning.
Internal previews