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

Prevent overwriting description of map fields #5158

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fredde-fisk
Copy link

In #3323, map fields where included in the OpenAPI specification, which is good. However, the documentation for the field might be confusing since it contains the following hard coded example that might be irrelevant and misleading for the one reading the documentation:

This is a request variable of the map type. The query format is "map_name[key]=value", e.g. If the map name is Age, the key type is string, and the value type is integer, the query parameter is expressed as Age["bob"]=18

It is also not possible to override this hard coded description using grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field, like this:

map<string, string> mapped_string_value = 1 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {
  description: "This will be ignored"
}];

This PR removes the hard coded description and instead let's the creator of the protobuf file decide the appropriate descriptions for the field.

References to other Issues or PRs

Have you read the Contributing Guidelines?

Brief description of what is fixed or changed

Other comments

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Yeah this makes sense to me. I think the old description was useful, but I agree it's probably confusing to the consumer of the API docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants