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

PubSub: Document FlowControl settings #8293

Merged
merged 1 commit into from
Jun 17, 2019
Merged

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Jun 12, 2019

Closes #7043.

This PR makes sure that FlowControl settings are properly documented in the generated documentation.

How to test

Steps to perform:

(from inside the pubsub/ directory)

  • Install the synthtool and run it,
  • Generate the pubsub docs locally:
    $ nox -f noxfile.py -s docs
    
  • Open the generated docs for FlowControl:
    $ chromium-browser "file://$GOOG_CLOUD_PYTHON_DIR/pubsub/docs/_build/html/types.html#google.cloud.pubsub_v1.types.FlowControl"
    

Expected result:
FlowControl settings have nice descriptions (as opposed to the current docs).

@plamut plamut added api: pubsub Issues related to the Pub/Sub API. type: docs Improvement to the documentation for an API. labels Jun 12, 2019
@plamut plamut requested a review from anguillanneuf June 12, 2019 14:07
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 12, 2019
@anguillanneuf
Copy link
Contributor

The generated docs look good to me.

Nit: I see the word "Property" in front of each property in the generated docs. Not a big deal, but would be great if we can make it consistent with the rest of the documentation.

@anguillanneuf anguillanneuf added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 12, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 12, 2019
@plamut
Copy link
Contributor Author

plamut commented Jun 12, 2019

The "property" prefix seems to be added to all namedtuple members, including those that do not have their docstrings modified, e.g. types.BatchSettings. It's probably the Sphinx itself that adds them, and the output might depend on the latter's version.

How are the current online docs generated and published anyway? Would a result be different if that was used instead of the local docs build?

FWIW, I get the same result (the "property" prefix) if building the docs locally on the master branch, making me believe that it's likely something unrelated to the changes in this PR ...

Copy link
Contributor

@anguillanneuf anguillanneuf left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement. type: docs Improvement to the documentation for an API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pub/Sub doc: missing definition for max_lease_duration in FlowControl
4 participants