Doc fixes (SC-857)#1330
Conversation
holmanb
force-pushed
the
holmanb/doc-fixes
branch
from
c6b662c to
88673ab
Compare
TheRealFalcon
left a comment
TheRealFalcon
left a comment
There was a problem hiding this comment.
Nice changes and good idea hiding deprecated instructions behind a dropdown.
However, I built this branch on rtd, and things aren't rendering correctly.
Left is current main, right is this branch. We lose bullets and some kind of formatting in multiple places.
Our version of sphinx is quite old, and I'm assuming the original reasons for limiting it no longer apply, so I'm happy to upgrade our version of it, but that may need to happen in a different PR in order to figure out the various quirks involved.
Deleting the old examples could be done without a version bump, but I believe the sphinx panels dependency and m2r2 depend on sphinx>2, so I'll try to get things fixed in this branch directly once I can test. |
holmanb
force-pushed
the
holmanb/doc-fixes
branch
from
9a4a0be to
6339b54
Compare
|
#1354 is probably relevant here. |
|
Or not...tried it with that PR rebased into this branch and still see the same rendering issues. |
holmanb
force-pushed
the
holmanb/doc-fixes
branch
2 times, most recently
from
269cad0 to
1e26e29
Compare
Thanks for the review @TheRealFalcon. I think I have things worked out now. It looks like this is a problem with specific version combinations of docutils and sphinx_rtd_theme. I've pinned the dependency versions for the bullet list rendering to work. I also noticed that the cli page had issues rendering cli flags with two hyphens inside italics, so I switched those to use inline codeblocks, which also look better anyways in my opinion. I don't see any other issues so I'm switching this back from draft to "ready for review". |
- m2r seems abandoned, switch to m2r2 - fix rst to actually render the security page
- Implying a cryptographic hash function is "safe" generally doesn't age well. - Linux users are likely to require escalating privileges on the host as part of system administration. Therefore, telling users to "only use ssh, not passwords" is likely to be interpreted as one of the following: - log in as root via ssh [1] - log in as a user that has passwordless sudo access [1] Both of these interpretations go against security best practices. Rather than giving security advice, warn of the risks associated with the feature and let users make their own decisions. [1] using an auth method such as ssh keys
They looked funny italicized currently, switch to inline codeblock.
holmanb
force-pushed
the
holmanb/doc-fixes
branch
from
1e26e29 to
ba95870
Compare
TheRealFalcon
left a comment
TheRealFalcon
left a comment
There was a problem hiding this comment.
This looks good now. Glad we're up to a non-ancient version of sphinx. Thanks!
3 participants
Note: it looks like docutils 0.17 / sphinx_rtd_theme < 0.5.2 breaks things, context can be found here (link included as a comment in doc-requirements.txt)
Planned future work: resolve the 40+ warnings that sphinx spews (mostly harmless, I think?) and then make warnings fail CI (this would have prevented some of the errors I fixed in this PR).