Improve docstrings to better explain args and functions #101
Conversation
| @@ -324,7 +324,7 @@ def _map_category(val: MapCategoryParams): | |||
| help="Number of parallel processes. Defaults to 1, set to 0 to use all cores") | |||
| def buildtables(library, outdir, ignoreoldstock, jobs): | |||
| """ | |||
| Build datatables out of the LIBRARY and save them in OUTDIR | |||
There was a problem hiding this comment.
I don't think this is an improvement "datatables" is term used across the codebase to mark tables with components. You just replaced "what" with type. The best would be to have both.
There was a problem hiding this comment.
What format is "datatables"? It should be documented. Is it a JSON, a CSV, or other? It should be indicated at every instance, as it's extremely ambiguous.
There was a problem hiding this comment.
It should be documented, but not here. For the user of the command (this string is what the CLI user sees) data table is something opaque.
It belongs to a system architecture description.
There was a problem hiding this comment.
Why don't you think it belongs in the doc strings?
A doc string should describe what a function does. The function generates JSON files.
There was a problem hiding this comment.
Abstraction and targeting who sees the message. The fact that datatables are encoded using JSON is just an implementation detail; it doesn't change how you use the function. When you use this function, you operate with datables as a black-box. It's like saying "Here, there is book." What I propose is to say "this book is a catalog of components", what you propose to say "It is in soft cover".
As I started - I think it is the best say both, but I strongly disagree that saying just "this is JSON" is useful. The most important thing to say is that it is a datable.
yaqwsx
force-pushed
the
master
branch
2 times, most recently
from
3dc9b5b
to
bb8f331
Compare
No description provided.