Skip to content

Propose docstring style for repo #272

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

Merged
merged 2 commits into from
Mar 27, 2025

Conversation

smaye81
Copy link
Member

@smaye81 smaye81 commented Mar 27, 2025

Here is a proposal for how to structure docstrings in protovalidate-python going forward (at least when porting the rest of the validation implementations). I think this strikes the right balance of adhering to best practices while not being onerous.

  • Functions being added to the library should get the bulk of the comment description in a docstring. No need to define Args:, Raises:, etc. though.
  • Any other public function, class, etc. should preferably have docstrings, though not necessary.
  • Be as succinct as possible while also keeping comments across implementations consistently worded.

For validation following an RFC / spec:

  • Always include any relevant ABNF grammar and keep aligned.
  • Indent grammar inside docstring.

One suggestion outside the scope of this PR might be to always have full docstring (Args, Raises, etc.) on any public method the user interacts with (i.e. stuff in validator.py)

@smaye81 smaye81 changed the title Docstring style proposal Propose docstring style for repo Mar 27, 2025
@smaye81 smaye81 changed the base branch from main to sayers/validation_upgrades March 27, 2025 01:16
@smaye81 smaye81 requested review from rodaine and timostamm March 27, 2025 01:18
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

👍

@smaye81
Copy link
Member Author

smaye81 commented Mar 27, 2025

@timostamm made even more consistent with other implementation comments here: 001449d.

is_uri_reference wouldn't pass a linter, but we can worry about that if we ever turn one on for docstrings.

@smaye81 smaye81 merged commit bb6012d into sayers/validation_upgrades Mar 27, 2025
7 checks passed
@smaye81 smaye81 deleted the sayers/docstrings branch March 27, 2025 12:52
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