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

Clarify meaning of the group name specified in WithGroupName #59784

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mikekistler
Copy link
Contributor

This PR makes a small change to the description of the value passed to WithGroupName to explain how it is used in OpenAPI document generation. The prior text was inaccurate because there is no "GroupName" in the OpenAPI specification.

@mikekistler mikekistler requested a review from halter73 as a code owner January 9, 2025 03:05
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Jan 9, 2025
@@ -115,7 +115,8 @@ public static TBuilder WithName<TBuilder>(this TBuilder builder, string endpoint
/// Sets the <see cref="EndpointGroupNameAttribute"/> for all endpoints produced
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also seems confusing -- what does it mean to "set an attribute"?

Maybe this should say "Sets the IEndpointGroupNameMetadata" ? or ???

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think this doc comment is attempting to apply a literal description of what this method is doing. It instantiates the EndpointGroupNameAttribute class and adds it to the metadata collection.

However, that's a means to an end for the real valuable thing which is the fact that EndpointGroupNameAttribute implements the IEndpointGroupNameMetadata interface. That's what's actually relevant here so I think your recommendation of referencing the interface instead of the class is a good one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants