doc: indent envvar bodies and add src-layout link #14181
Conversation
Indent the body text of all ``.. envvar::`` entries in ``reference.rst`` to match the 3-space indentation convention used by ``.. confval::`` entries in the same file. Add a ``.. seealso::`` link to the packaging.python.org src-layout vs flat-layout discussion in ``goodpractices.rst``. Follow-up to pytest-dev#14179.
webknjaz
left a comment
webknjaz
left a comment
There was a problem hiding this comment.
@him2him2 I recommend you to learn about the concept of atomic changes in Git. This is something that prevents the PR from being merged right now. If this was in two standalone/atomic PRs, I'd merge one of the changes right away. But now we have to wait. Making atomic submissions makes sure that things that need more work don't block changes that are usable already.
|
|
||
| .. seealso:: | ||
|
|
||
| `src layout vs flat layout <https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/>`_ |
There was a problem hiding this comment.
It's a Sphinx site. We can just use intersphinx. Also, I was thinking of just linking the existing text. Not sure how others will feel about having this here. Let's see...
There was a problem hiding this comment.
Good point on both counts — switched to intersphinx (:doc:packaging:discussions/src-layout-vs-flat-layout) in the replacement PR. Happy to rework to an inline link if that's preferred.
There was a problem hiding this comment.
Looks like the render/structure got better: https://pytest--14181.org.readthedocs.build/en/14181/reference/reference.html#environment-variables. So the changes to this document improve it and I don't have any objections. But the changes in other file are up for discussion.
Tip
@him2him2 you may want to contribute a better example of using the .. envvar:: directive to Sphinx's own docs — they don't seem to make it clear that the directive accepts both the env var name and a blurb with explanation.
There was a problem hiding this comment.
Thanks — glad the rendering improved. Good idea about contributing a better envvar example upstream to Sphinx, I'll look into that.
There was a problem hiding this comment.
Followed up on the Sphinx suggestion — opened sphinx-doc/sphinx#14295 to add an example to the .. envvar:: directive docs, following the same pattern as .. confval::. Thanks for the nudge!
There was a problem hiding this comment.
Great! This practice is most common among maintainers and people who ended up gaining expertise in specific areas that are universally useful in many projects — trying to improve upstream and replicate similar contributions across many projects. This often results in a greater impact on the ecosystem and tooling.
webknjaz
added
type: docs
|
Closing in favour of two atomic PRs per @webknjaz's feedback:
Thanks for the review and the note about atomic commits — lesson learned. |
|
@webknjaz Thanks for taking the time to review and for the guidance on atomic commits and intersphinx — it was a pleasure working with you today! |
@him2him2 no problem! For the future, you don't have to close the original PR since it contains context. You could create one other PR and use interactive rebase to re-shape the branch on the first one with a force-push so it'd be accepted. It's usually a good idea to keep related context together. |
2 participants
Follow-up to #14179 per @webknjaz's review comments.
Changes
reference.rst: Indent the body text of all 14.. envvar::entries with 3 spaces, matching the.. confval::convention used in the same file. This was the only section with unindented directive bodies.goodpractices.rst: Add a.. seealso::link to the packaging.python.org src-layout vs flat-layout discussion after the note block about layout choices.