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.

Sign up for GitHub

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

Jump to bottom

Adding check that all public modules are documented. #1375

Merged
merged 3 commits into from
Feb 12, 2016
Merged

Adding check that all public modules are documented. #1375

merged 3 commits into from
Feb 12, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jan 9, 2016

Fixes #714.

Also: making prints to stderr Python 3 friendly in run_pylint.py.

@tseaver Note the docs rule will fail here. This was intentional. I wanted to display which modules were still undocumented so we could discuss if they should be added to the IGNORED_MODULES set or if we should add them to an rst file.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 9, 2016
@dhermes dhermes force-pushed the check-left-out-modules branch from 3adfee2 to 8caffb9 Compare January 15, 2016 07:33
@dhermes
Copy link
Contributor Author

dhermes commented Jan 15, 2016

@tseaver PTAL

tseaver
tseaver reviewed Jan 20, 2016
View reviewed changes
scripts/verify_included_modules.py
OBJECT_INVENTORY_RELPATH = os.path.join('_build', 'html', 'objects.inv')
IGNORED_PREFIXES = ('test_', '_')
IGNORED_MODULES = frozenset([
'gcloud.bigquery.query',

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes dhermes force-pushed the check-left-out-modules branch from 8caffb9 to 8285be2 Compare January 20, 2016 18:53
@dhermes
Copy link
Contributor Author

dhermes commented Jan 20, 2016

@tseaver PTAL. This time docs should fail with an explanation of which modules we leave out.

At this point we should discuss which need to be actually documented and which I should add to IGNORE_MODULES.

When running locally, the errors are:

Found undocumented public modules:
- gcloud.bigquery.query
- gcloud.datastore.helpers
- gcloud.environment_vars
- gcloud.iterator
- gcloud.storage.batch
- gcloud.streaming.buffered_stream
- gcloud.streaming.exceptions
- gcloud.streaming.http_wrapper
- gcloud.streaming.stream_slice
- gcloud.streaming.transfer
- gcloud.streaming.util

@dhermes
Copy link
Contributor Author

dhermes commented Jan 28, 2016

@tseaver PTAL

@dhermes dhermes force-pushed the check-left-out-modules branch from 8285be2 to c04d18d Compare January 28, 2016 23:06
@tseaver
Copy link
Contributor

tseaver commented Jan 29, 2016

LGTM

@dhermes
Copy link
Contributor Author

dhermes commented Jan 29, 2016

@tseaver There are still questions to resolve.

We have several undocumented modules and need to decide which ones to ignore and which ones need documenting.

I expected the CI to fail but tox -e docs only gets run on merged commits or tagged commits (not on PRs).

@dhermes
Copy link
Contributor Author

dhermes commented Feb 3, 2016

@tseaver PTAL, pending question above

@dhermes
Adding check that all public modules are documented.
60bec76 
Fixes googleapis#714.
@dhermes
Addressing review comments.
5c90953 
- Removing accidentally including bigquery module from
  IGNORED_MODULES in verify_include_modules
- Adding Sphinx as a lint dependency (it couldn't be imported)
- Fixing Python 2 style print statements in run_pylint
@dhermes dhermes force-pushed the check-left-out-modules branch from c04d18d to 722d6ab Compare February 12, 2016 21:33
@dhermes
Updating included docs and ignoring some that need not be public.
27da413 
Also making sure `tox -e docs` **always** gets run in update_docs.sh,
to verify the docs build successfully at all times.
@dhermes dhermes force-pushed the check-left-out-modules branch from 722d6ab to 27da413 Compare February 12, 2016 21:44
@theacodes
Copy link
Contributor

I think we should ignore:

  • gcloud.streaming.*

But we should probably document all of these:

  • gcloud.bigquery.query
  • gcloud.datastore.helpers - unless there's nothing in here useful to users.
  • gcloud.environment_vars
  • gcloud.iterator - unless it's thoroughly documented by its downstream clients.
  • gcloud.storage.batch

@dhermes
Copy link
Contributor Author

dhermes commented Feb 12, 2016

Everything except gcloud.iterator was documented in the latest commit. It definitely should not be user facing, I'm not sure why we made it public.

@theacodes
Copy link
Contributor

Cool, LGTM.

dhermes added a commit that referenced this pull request Feb 12, 2016
@dhermes
Merge pull request #1375 from dhermes/check-left-out-modules
d56ea3d 
Adding check that all public modules are documented.
@dhermes dhermes merged commit d56ea3d into googleapis:master Feb 12, 2016
@dhermes dhermes deleted the check-left-out-modules branch February 12, 2016 22:28
@dhermes dhermes mentioned this pull request Feb 17, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@theacodes theacodes

Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dhermes @tseaver @theacodes @googlebot