-
-
Notifications
You must be signed in to change notification settings - Fork 25
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
Tidy up settings in docs/
from initial templating
#27
Conversation
538c64b
to
dc68b35
Compare
I also noticed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With regards to the removals in the conf.py
file, I can understand eliding stuff this repo will never use, like the output sections for man pages, epub, etc, but is there a reason to remove options that could plausibly be used in the future, and most of the descriptive comments, even for the options we are using? Space is not at a premium here, and in-use options are clearly indicated by the syntax highlighting, so I'm not sure the simple desire for concision justifies the removal of settings and comments that could be helpful to others in the future.
(Minor copyediting nit, since it will become the commit message with a squash merge—just like with dashes and hyphens, always use an balanced number of spaces (usually zero) on either side)
Sidenote: @encukou It would be helpful for docs community members (or whatever we're officially called) to have at least triage access to this repo, to be able to assign themselves to issues, tag reviewers on PRs, fix titles, manage labels, etc.
I remember a similar discussion for the Sphinx template file iteself. The argument I personally subscribe to is (a) that the comments are clearly for initially filling in the values ("Do X" rather than "X does") and as such at least should be updated to a descriptive style, and (b) those updating these values should look to the official Sphinx documentation, and comments in the source can lead one into a trap where you don't bother and then waste more time as you take the comment as authoritative, whereas it may be out of date, oversimplify, or just is wrong.
What's this referring to, sorry? A |
Right, but the nominal conventional style is for comments to be imperative, albeit with the "thing" being described as the subject of the sentence rather than the user—that aside, the main point is the substantial loss of information in the comments, in whatever phrasal form one prefers.
True; that's a good point, as these files can drift out of date and be a real chore to update—I've faced that firsthand with various config files like this. On the other hand, at least within a major version, one would think compatibility constraints would entail at least these high-level descriptions still being accurate, i.e. an existing option changing syntax or semantics within a major version would be a significant backward-compatibility breakage that would be precluded by semver and most compatibility policies.
"docs/ settings [sic]" in the PR title, sorry, which becomes the squashed commit summary by default |
Would "Tidy up A |
I'm 👍 on either one, thanks |
docs/
from initial templating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a non-blocking suggestion: In the conf.py file, mirror the sections in Sphinx’s documentation, separating sections with a separator and adding relevant URLs for each section.
See https://github.com/pradyunsg/installer/blob/main/docs/conf.py for what this looks like, in practice. sphinx-doc/sphinx#10056 has me trying to convince Sphinx maintainers to change the default conf.py template to this style.
That's a great suggestion @pradyunsg , and one that dovetails well with @AA-Turner 's reasoning as well—I think that yet again, you've (mostly) convinced me. I do have one major reservation with your broader proposal, though—I've found time and again that having literal examples of the option keys and values that can be used as a base is often invaluable not only for convenience, but in clarifying ambiguities, lack of clarity and missing/assumed details in the prose description that the tool authors might not even have thought of (which I've been on both sides of). Unfortunately, this is not consistently present in the Sphinx docs linked to; some options have examples, but the great majority don't. This is a pretty critical loss of information; if sphinx-doc/sphinx#10056 is adopted, the examples for each setting currently present in the sample Its just that often, edge cases or ambiguity that may seem obvious or implicit to tool maintainers is in fact not so to readers, which examples very often clarify; furthermore, many users learn faster and better by example than by reading a chunk of prose, and they are also much more copyable, to reduce the increased mechanical burden and decreased convenience of not having commented examples in the file. Perhaps this belongs more on the other thread (since it isn't so critical here in this specific case)? |
Hmm, why remove the Makefile? That's what I use to build docs...
I agree, here you go: https://discuss.python.org/t/documentation-triagers/13850 |
I debated back and forth as to whether to mention that as well, but ultimately decided not to. It appears the intention of the Sphinx developers, per sphinx-doc/sphinx#5618 , is to remove the auto-generated makefiles, store build options in the config and expose a unified |
Could we please keep the Makefile until that |
The Makefile is out of date anyway (references to "TeamCompass"). When you invoke make for docs, what command do you run? (I'm on windows so make is another world). Adding a makefile that passes through to a python script may be the best of both, allowing easy invocation for windows users too. A |
|
I added a simple A |
If the problem is the fact that the project name says TeamCompass, you can fix that in a +1/-1 change. Similarly, if you want to have top level Makefile, you can set I'm not sure we need to be creating a custom Python file that imports things, hardcodes While I think cleaning up the conf.py file is a good idea; I'm not too sure that it's a good idea to replace/rework the default Sphinx Makefile with something bespoke. |
In the past when I've used I hadn't realised that this was the default makefile (again, I don't use them) -- would it be better to simply not touch the files at all in this change? People feel rather strongly about them, it seems! A |
Reverted the makefile changes; added comments to conf.py. A |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, are there any blockers?
None I'm aware of. A |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @AA-Turner
This removes a lot of commented code from conf.py, and removes the template makefiles
A