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

fix: list oneofs in docstring #1030

Merged
merged 7 commits into from
Oct 18, 2021
Merged

fix: list oneofs in docstring #1030

merged 7 commits into from
Oct 18, 2021

Conversation

busunkim96
Copy link
Contributor

@busunkim96 busunkim96 commented Oct 13, 2021

Fixes #713

  • FIeld attributes state which oneof they are part of.

  • If a message has a oneof with more than 1 member field, the docstring includes a short explanation of oneofs and a list of the oneofs in the message.

The list looks a bit odd in the generated docs (output of nox -s docs / what's published to googleapis.dev) - suggestions on better ways to present the information are very appreciated.

@dandhlee Do you have an idea of how this docstring change would look in Cloud RAD?

image

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 13, 2021
Comment on lines 8 to 11
This message has the following `oneof`_ fields (mutually exclusive fields).
For each oneof, at most one member field can be set at the same time.
Setting any member of the oneof automatically clears all other
members.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -512,13 +512,26 @@ class DeleteFeedRequest(proto.Message):
class OutputConfig(proto.Message):
r"""Output configuration for export assets destination.

This message has the following `oneof`_ fields (mutually exclusive fields).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dandhlee This is a concrete example of the new docstring with the oneof information.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will take a look, intuition tells me it'll just show up as `oneof`_

Copy link
Contributor

Choose a reason for hiding this comment

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

Sadly it doesn't get parsed, but I'll be happy to include support for it C:

@busunkim96 busunkim96 marked this pull request as ready for review October 14, 2021 14:45
@busunkim96 busunkim96 requested a review from a team as a code owner October 14, 2021 14:46
busunkim96 and others added 4 commits October 14, 2021 22:33
Merge branch 'oneofs-in-docstrings' of github.com:googleapis/gapic-generator-python into oneofs-in-docstrings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docstrings do not specify which fields are one-ofs
5 participants