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

Generate documentation of private members through sphinx docs #2831

Merged

Conversation

barthisrael
Copy link
Collaborator

With this commit we make sphinx build API docs for the following things, which were missing up to this point:

  • __init__ method of classes;
  • "private" members (properties, functions, methods, attributes, etc., which name starts with an underscore);
  • members that are missing a docstring, so we can still reference them with links in the documentation.

The third point can be removed later, if we wish, when we reach a point where everything has proper docstrings in the Patroni code base.

References: PAT-195.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5964715633

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 99.862%

Totals Coverage Status
Change from base Build 5928016358: 0.0%
Covered Lines: 13045
Relevant Lines: 13063

💛 - Coveralls

@hughcapet
Copy link
Member

what's with the Tests / docs step failing here?

@matthbakeredb
Copy link
Contributor

matthbakeredb commented Sep 4, 2023

what's with the Tests / docs step failing here?

I expect sphinx wasn't parsing the private methods at all before so didn't detect formatting problems. I'll take a look.

@@ -257,7 +257,7 @@ def __init__(self, configfile: str,

* file or directory path passed as command-line argument (*configfile*), if it exists and the file or
files found in the directory can be parsed (see :meth:`~Config._load_config_path`), otherwise
* YAML file passed via the environment variable (see :cvar:`PATRONI_CONFIG_VARIABLE`), if the referenced
* YAML file passed via the environment variable (see :attr:`PATRONI_CONFIG_VARIABLE`), if the referenced
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, in the class docstring it is declared as :cvar:. Shouldn't it be here the same?

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently, it's not a valid "domain" according to sphinx command output. Docs are here for what can be used
https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#cross-referencing-python-objects

Comment on lines 283 to 288
Load validators from YAML files that were found.

Recursively walk through ``available_parameters`` directory and load validators of each found YAML file into
``parameters`` and/or ``recovery_parameters`` variables.

Walk through directories in top-down fashion and for each of them:
* Sort files by name;
* Load validators from YAML files that were found.
Copy link
Member

Choose a reason for hiding this comment

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

Is it an expected change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmn yes that is odd. Let me revert it and see if there were errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, unrelated. Reverted.

barthisrael and others added 3 commits September 7, 2023 12:44
With this commit we make sphinx build API docs for the following
things, which were missing up to this point:

* `__init__` method of classes;
* "private" members (properties, functions, methods, attributes, etc.,
  which name starts with an underscore);
* members that are missing a docstring, so we can still reference
them with links in the documentation.

The third point can be removed later, if we wish, when we reach a
point where everything has proper docstrings in the Patroni code base.

References: PAT-195.

Signed-off-by: Israel Barth Rubio <[email protected]>
…hinx

* `:cvar:` is not a valid domain role. Replaced with `:attr:`.
* documentation for `consul.base.Consul.__init__` has a single backtick
quoted string which is interpreted as a reference which cannot be found.
Therefore, the docstring has been copied as a block quote.
* various list spacing problems and indentation problems.
* code blocks added where indentation is interpreted incorrectly
* literal string quoting issues.
@matthbakeredb matthbakeredb force-pushed the feature/sphinx-doc-private-members branch from 78a4b15 to 1e38a7a Compare September 7, 2023 11:45
@CyberDem0n
Copy link
Member

👍

1 similar comment
@hughcapet
Copy link
Member

👍

@hughcapet hughcapet merged commit a2ceff1 into patroni:master Sep 11, 2023
23 checks passed
@hughcapet hughcapet added this to the Sprint 2023.17 milestone Sep 11, 2023
@barthisrael barthisrael deleted the feature/sphinx-doc-private-members branch September 11, 2023 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants