-
Notifications
You must be signed in to change notification settings - Fork 57
fix(internal/gengapic): add API version to client comment #1677
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
Conversation
|
I guess my main question here is how much value we're getting from the repetition of the docstring? Should we just add a single comment? Just the comment on the the struct type or in the generated file headers? It's not currently exposed in as a constant or method so there's no programatic access to the identifier, but I'm not clear if that will change. This is more nitpicky, but can/should we describe this value better with the comment? As a casual reader I already potentially have multiple endpoints that are versions (apiv1, apiv1beta1, etc), and module versions so a more descriptive comment would be helpful here to describe why users now have yet another versioning concept to keep track of. |
Yeah, just on the struct type is fine. I was just plugging into the doc gen function used for clients and its used in multiple places and didn't think it was that disruptive. I'll just add a param to the function and only have it on the
I don't think that will change unless there is a clear use case for having programmatic access to the value (don't want to over expose when we don't need to, ya know?). ATM there is no such use case.
Yeah that's a fair ask. I think this comment, on the client type, should be tight and "do a job" i.e. be the breadcrumb users need to correlate this client with other API artifacts e.g. documentation site(s). I am working on another PR for package level documentation (in doc.go) that will list all client-interface-version tuples as a sort of ToC for API versions - would putting a brief description of what this is in that heading suffice? |
Thanks!
Most of the usage I imagine is more exotic versions of dependency resolution, so tools can find a module release that supports a given interface contract version. I think its fair to keep it out of the mix.
I think that's a great way to handle it, thanks! |
Include the API version for the interface (protobuf
service) client on the client documentation.AIP-4236 denotes this a "may" requirement, but I am updating it to a "must" requirement for all languages in aip-dev/google.aip.dev#1585
Motivating bug http://b/467065424. Task tracking bug http://b/467067975.