Talk:Spicerack/Cookbooks

From Wikitech
Jump to navigation Jump to search

Proposal: relax the docstring requirement

Currently we configure pep257 with prospector to require docstrings for modules, classes, and all public methods. This leads to a proliferation of docstrings like

   def argument_parser(self):
        """As specified by Spicerack API."""

and

   def __init__(self, args, spicerack):
       """Initialize the runner."""

See for example cookbooks/sre/aqs/roll-restart.py.

These comments take time to write and read, and do not document the unique behavior of the cookbook. Effectively they are documenting the spicerack API requirements, which should go in the spicerack documentation, rather than going into every cookbook.

Instead of this strict requirement, I recommend we normalize requesting documentation for cookbooks when we request reviews, and document an example cookbook that effectively uses docstrings to communicate its behavior.

We could potentially require a subset of the docstrings, such as requiring module docstrings, but requiring every module, class, and method to have a docstring is leading to too much boilerplate.

Razzi (talk) 22:42, 7 February 2022 (UTC)[reply]

While some cookbooks are simple enough, some other are quite complex and have many methods that benefit from a sane docstring explaining what they are doing and documenting their parameters. Regarding your first example, yes I agree the docstring is superflous and it would be nice that the library could recognize inherited methods and allow to not alert for those. There is a issue open upstream, see https://github.com/PyCQA/pydocstyle/issues/309
As for your second example, the signature is totally free format, you could design your cookbook to not accept args but directly some parameters and get_runner() passing them from the args. In that case it would make total sense to have a proper docstring.
In my opinion relying on people in code reviews to remember to check for docstrings and ask for them seems to me a step back from where we're now, and I judge the overhead of having docstrings actually small but I'd like to hear also other's opinions.
Volans (talk) 09:47, 8 February 2022 (UTC)[reply]