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

Add published API documentation #24

Merged
merged 34 commits into from
Jun 26, 2020
Merged

Add published API documentation #24

merged 34 commits into from
Jun 26, 2020

Conversation

vinceatbluelabs
Copy link
Contributor

@vinceatbluelabs vinceatbluelabs commented May 16, 2020

Docs published here! https://db-facts.readthedocs.io/en/published_docs/

This adds public API documentation only; the existing markdown visible in GitHub is not part of things yet. I'm inclined to wait until after records-mover 1.0 to worry about that.

@vinceatbluelabs vinceatbluelabs changed the title First import of documentation framework Add published documentation May 16, 2020
docs/source/conf.py Outdated Show resolved Hide resolved
"""Get connection info for specified database."""
"""Get connection info for specified database.

:param db_name: Alias for the particular database endpoint and account to connect to. ['a','b','c'] corresponds to 'a-b-c' on the db-facts command-line.

Choose a reason for hiding this comment

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

[flake8] E501 line too long (157 > 100 characters)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I punted on this one; I couldn't figure out how to add a continuation to a directive in RST. Maybe it's worth flipping over to google format docstrings? Not sure if this is solvable there or not, just can't remember if you had a take on that. I think the hypermodern series uses google format instead of RST.

configured database named 'redshift'. Other parts of this chain can
be found in the [ws-scripts](https://github.com/bluelabsio/ws-scripts)
repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops! We still had a reference to an internal repo. Getting this stuff packaged up and open sourced is in the backlog, but not on the critical path to get 1.0 of records-mover out.

'db',
'DBFacts',
'UserErrorException'
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using __all__ helps clarify the public API and also gives Sphinx the hint to put the documentation for these in the overall package. These are pretty fundamental, so I'm more comfortable having them in the top-level package than I am with the names of the subpackages they currently live in :)

.. automodule:: db_facts
:members:
:undoc-members:
:show-inheritance:
Copy link
Contributor Author

@vinceatbluelabs vinceatbluelabs Jun 22, 2020

Choose a reason for hiding this comment

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

This is hand-tuned to show the bits we want to document - the public API as well as the details about the provided jinja contexts. Having the rest marked private with a leading underscore may allow us to have this file be auto-generated so we won't have to tweak it if more public API is added in the future--I may play around with that and see what the amount of effort is there.

# Catch-all target: route all unknown targets to Sphinx using the new
# "make mode" option. $(O) is meant as a shortcut for $(SPHINXOPTS).
%: Makefile
@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is stock from the auto-generated template so far.

# The default sphinx doesn't seem to set TYPE_CHECKING, which
# results in dropping our use of TypedDict, Literal and other
# things we don't use by default to avoid extra dependencies.
'sphinx_autodoc_typehints',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The net result from sphinx_autodoc_typehints could be better - but it's definitely better than seeing the detailed return value from db() as just dict.

# Ensure TYPE_CHECKING is set to True so we document using types from
# typing_extensions (e.g., TypedDict)
#
set_type_checking_flag = True
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this section and flag.

@vinceatbluelabs vinceatbluelabs marked this pull request as ready for review June 24, 2020 15:02
@vinceatbluelabs vinceatbluelabs changed the title Add published documentation Add published API documentation Jun 24, 2020
@vinceatbluelabs vinceatbluelabs requested a review from cwegrzyn June 24, 2020 15:06
@vinceatbluelabs vinceatbluelabs merged commit 1e97cba into master Jun 26, 2020
@vinceatbluelabs vinceatbluelabs deleted the published_docs branch June 26, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants