-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
New Sphinx tutorial, part III #9534
New Sphinx tutorial, part III #9534
Conversation
|
https://sphinx--9534.org.readthedocs.build/en/9534/tutorial/describing-code.html is ready for review. |
Could you divide this PR into two PRs; 1) separating a document into multiple files and 2) adding a new document, please? It helps to read the diff. I need to read the whole of the text if they're mixed. |
FYI: The CI error will be fixed if you merge the HEAD of 4.x branch into your branch. |
This comment has been minimized.
This comment has been minimized.
I know. But it's a bit annoying if reviewing continues long. Additionally, no advantage to keeping them coupled. |
Separated the split in #9540, this should be reviewed after that. |
ca8cf87
to
0c41529
Compare
This is now ready for review! |
This warning is on me to fix, but again the filename has a double extension:
|
11c857f
to
61ecb72
Compare
61ecb72
to
a606db0
Compare
Since nobody has started reviewing this yet, and in the interest of cleaning up the git history now that #9540 has been reviewed, I rebased and force pushed this. |
Comment for future reviewers: I wonder if I should mention |
I don't believe so. It's not well-tested, and should be a bit more robust before it's in the official tutorial. |
Sorry, I meant the |
Reminder that this is ready for review ✔️ |
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.
I got one of the files reviewed, at least. I ran out of energy before I got to the second one.
Co-authored-by: Manuel Kaufmann <humitos@gmail.com>
I addressed the comments, and also added an explanation at the top stating that several languages are supported. This is ready for the final review. |
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.
The new text about the languages is great, but I'm still concerned about the structure. The current structure is
so let's say someone writes a C version tomorrow, how should the structure be changed? Having more subsections under "Describing code in Sphinx" seems confusing, even if the doctests section gets "Python" in its title. |
One idea could be to add a "Documenting other languages" subsection, or appendix, to this chapter. That way it doesn't "break the flow" of the current tutorial, but I doubt that it will be sufficient for avid C and C++ users. Another idea would be to turn this chapter into a sort of "Choose your own adventure" thing, in which Lumache is either written in Python, C, or C++. Some minor wording modifications would be needed in the introduction, but apart from that, the prose would be almost the same. And another idea would be to write a "Sphinx for C and C++ users" tutorial (I don't think there has to be one tutorial, or the tutorial - a tutorial is just another category of technical documentation) that assumes that the user has read the first sections of the Beginners tutorial (the one we're writing in this series of pull requests) and picks from there. That would require a bit more work, but possibly provide a more pleasant reading experience for C and C++ users. |
I'm not too keen on this. As a non-Python programmer you are then directed towards Python stuff you may not be interested in or need at all. And while Sphinx has an origin story with CPython, it is used much more widely now.
(Emphasis mine) Exactly! This is what I would most like. You have nicely written the tutorial such that this is possible (except the present structuring discussion), so why not reuse the rest of the tutorial for non-Python programmers?
to
and add the choose-your-adventure sentence? But perhaps that section nesting is too deep? That's why I think it should be thought through now rather than later.
I'm not sure that work would be worth it. I don't think there is anything language specific in the previous sections (except for may be intro notes in the beginning about the overall structure), so essentially duplicating them doesn't seem right to me. |
For the "choose your own adventure" option, we could use tabs, but perhaps some minor edits in the prose would be needed. Not too keen on adding 2 extra subsections. For the last option, just to clarify, the first chapters wouldn't be repeated. The tutorial would announce that "if this is your first time with Sphinx, read the first N chapters of the basic tutorial before this one". In any case, I'm a fan of YAGNI, and I'd rather not modify the present PR for something that is not there yet. We don't know if the C & C++ versions will come in the next month, or we will have to wait for 3 years... |
I don't think it's important to describe all programming languages Sphinx supports in this tutorial. The important thing I think is letting the readers know 1) what domains can do (describing object and referencing them), 2) where the directives and roles are described, and 3) the difference of the languages. This chapter only explains about the usage of But, this tutorial does not mention about 3). Autodoc and doctest processes only Python codes. Other languages are not supported. So it would be much better to explain the difference. So I feel this would be better:
What do you think? Note: I think it's not too late to make ToC extendable after merging. I'd like to merge this if we'll get agreed on the structure of ToC. It's okay to modify in another PR :-) |
For me this proposal is good, it's more or less aligned with the first idea I wrote in #9534 (comment), but with a better structure so it's more clear what differences are between the languages. |
I like it as well, it's extensible without requiring large changes up front. |
We have a plan then? Is this ready to merge? :) |
Do you have a plan to change text about this (in another PR)? |
Except above comment, I'm okay to merge this. @jakobandersen Do you have any opinion? If not, I'll merge this soon. |
Sure, count on it 👍🏽 |
Friendly ping :) |
Okay, let's go forward! |
Thanks! Setting a reminder to myself to address the changes discussed here. |
Feature or Bugfix
Purpose
This is a continuation of the work we did in #9276 and #9355. It expands some sections already started in the first part and adds some new ones, as originally proposed in #9165:
In #9424 I started writing a section on intersphinx, but after some discussion I decided to postpone it a bit.
Also, as suggested by @tk0miya in #9424, this splits the tutorial into several documents, to make it more amenable.
Rendered version: https://sphinx--9534.org.readthedocs.build/en/9534/tutorial/index.html
cc @ericholscher
Relates