docs: Adds a guide to writing code #751
Conversation
a-ovchinnikov
force-pushed
the
python_coding_guide
branch
from
884c96a to
35632dd
Compare
|
Shouldn't be this part of CONTRIBUTING.md ? |
Should be linked there at least |
|
I would not mix docs for package managers and coding best practices 🤷 |
Ideally not. Contributing guidelines are already quite bloated in content, so stylistic things should go to something like "HACKING.md"
Yes, agreed. |
docs/coding_best_practices.md
Outdated
|
|
||
| ## FAQ | ||
|
|
||
| Q. Is this guide exhaustive? |
There was a problem hiding this comment.
This doesn't render well
a-ovchinnikov
force-pushed
the
python_coding_guide
branch
3 times, most recently
from
37e2b22 to
7a4e3be
Compare
|
|
||
| ### Write easy to follow code | ||
|
|
||
| - Try avoiding extremely long lines in any direction. Horizontal lines longer |
There was a problem hiding this comment.
We have a linter rule that covers this. Is it necessary to add this point here as well?
There was a problem hiding this comment.
Horizontal -- yes, vertical -- not really. I'd have kept it for completeness.
| - Consider variable names which reflect intended use and potentially types of | ||
| objects they will be bound with. | ||
|
|
||
| - While doing the above remember, that vowel shortage is over, so in most |
There was a problem hiding this comment.
I'm not sure I completely get what you mean here. Can you add a quick example?
There was a problem hiding this comment.
lst_pckgs and other overly compacted names. This is not a problem in the codebase yet and arguably I am the one most prone to using them (only in very narrow scopes, but nevertheless).
| * [Write clear tests which actually test functionality](#write-clear-tests-which-actually-test-functionality) | ||
| * [FAQ](#faq) | ||
|
|
||
| ## Background |
There was a problem hiding this comment.
Altough I think this section is very well written and is pertinent, I'm afraid that the large text might scare off some people from reading. I feel like the two first paragraphs have the most important messages (we enforce good standards for maintainability, and reviews are done in good faith).
In any case, I'm not strongly against keeping this section as is, so please do so if you prefer.
There was a problem hiding this comment.
I was taking a wider look at both CONTRIBUTING.md and coding_best_practices.md, and they might be a little too much information for a newcomer. Maybe we can add some quick reference summaries later on, so one can get up to speed quickly.
There was a problem hiding this comment.
This is intended to be more of a supplementary text and not a must-read. All I do here is reiterate basics. I'll reword the referring sentence in CONTRIBUTING.md to highlight this.
| and [PEP20](https://peps.python.org/pep-0020); | ||
|
|
||
|
|
||
| ### Write clear tests which actually test functionality |
There was a problem hiding this comment.
Should we drop the test guidelines from CONTRIBUTING.md?
There was a problem hiding this comment.
No. That guide is the main one, this is supplementary.
|
|
||
| ## Best practices | ||
|
|
||
| ### Document your decisions |
There was a problem hiding this comment.
Should we drop the comment guidelines from CONTRIBUTING.md?
There was a problem hiding this comment.
I think no. The idea is that CONTRIBUTING.md is enough for most if not all contributors, and this is a fall-back for cases when we need to explain why certain piece of code does not pass a code review.
a-ovchinnikov
force-pushed
the
python_coding_guide
branch
2 times, most recently
from
e8c0973 to
a82a3e6
Compare
This commit adds a reference document with some best practices to help resolve disputes when writing code. Related to: containerbuildsystem#737 Signed-off-by: Alexey Ovchinnikov <[email protected]>
a-ovchinnikov
force-pushed
the
python_coding_guide
branch
from
a82a3e6 to
ea474d7
Compare
|
I really like what's here so far (not least because I strongly agree with all of it 🤣). Have you considered adding a link to The Zen of Python? I don't know how well-known it is these days, but IMO it's an invaluable resource (outside of Python, even). |
Yes, I have provided a link to 19 of the 20 postulates. Edit: L201, GH keeps rendering the page with the highlight just off screen. |
LOL, I had forgotten that it started life as a PEP 😆 |
This commit adds a reference document with some
best practices to help resolve disputes when
writing code.
Related to: #737
Maintainers will complete the following section
Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:
/ok-to-test(as is the standard for Pipelines as Code)