-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
docs: include documentation for required parameters #964
Conversation
Currently it is unclear how to use some of the functions that return calls. Their required parameters are undocumented. This means a user of the library either needs to go look at the official docs page for the api or find the corresponding do method in the code and look for its private discovery json comment. This commit starts to add parameter descriptions for required fields, those that are passed to the Call method. Fixes: #950
@tbpg The first commit contains the actual code changes to the generator. LMK what you think of the formatting. I am open to suggestions! |
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.
Looks great!
// - name: The resource name of the settings. Format is one of: * | ||
// "projects/{project}/accessApprovalSettings" * | ||
// "folders/{folder}/accessApprovalSettings" * | ||
// "organizations/{organization}/accessApprovalSettings" |
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 we make this show up properly?
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 there is too much we can do here. It is hard to know intent of *
. I don't think you can confidently tell where the last bullet should end. This is also a problem in other struct field and method docs already today.
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.
LGTM.
Thanks |
Currently it is unclear how to use some of the functions that
return calls. Their required parameters are undocumented. This
means a user of the library either needs to go look at the official
docs page for the api or find the corresponding do method in the
code and look for its private discovery json comment. This commit
starts to add parameter descriptions for required fields, those
that are passed to the Call method.
Fixes: #950