Improve documentation on contributing to documentation #8082

cocobennett · 2021-04-09T01:06:12Z

astrojuanlu · 2021-04-09T11:27:09Z

Hi @cocobennett , thanks for your interest in contributing to Read the Docs! 😊

I notice that this PR is still marked as draft, so let us know when it's ready to receive feedback.

Also, I noticed that the branch contains a spurious merge commit, I think it would be better if you rebase on top of readthedocs:master and force push, to keep the history as clean as possible.

cocobennett · 2021-04-09T17:01:47Z

Hi @cocobennett , thanks for your interest in contributing to Read the Docs! 😊

I notice that this PR is still marked as draft, so let us know when it's ready to receive feedback.

Also, I noticed that the branch contains a spurious merge commit, I think it would be better if you rebase on top of readthedocs:master and force push, to keep the history as clean as possible.

Thanks @astrojuanlu, you're welcome and I'm happy to help! Right now, I'm trying to figure out how to contribute locally so I can write up those instructions and will let you know!

And great tip, thank you. I will rebase instead

cocobennett · 2021-04-09T17:19:08Z

@astrojuanlu Actually, if you could help me, I am having one issue. I've followed the instructions here https://docs.readthedocs.io/en/stable/development/install.html (including the read-only settings)

but when I navigate to http://community.dev.readthedocs.io/, it looks like this where it's not formatted correctly. Have you seen this before?

stsewd · 2021-04-09T18:27:28Z

@astrojuanlu Actually, if you could help me, I am having one issue. I've followed the instructions here https://docs.readthedocs.io/en/stable/development/install.html (including the read-only settings)

when editing the policy you need to leave the "prefix" field empty and click on add. After your first build you'll see that there is another bucket named "media" and you'll need to do the same. We should make this more clear with a screenshot maybe

cocobennett · 2021-04-09T20:06:53Z

@astrojuanlu Actually, if you could help me, I am having one issue. I've followed the instructions here https://docs.readthedocs.io/en/stable/development/install.html (including the read-only settings)

when editing the policy you need to leave the "prefix" field empty and click on add. After your first build you'll see that there is another bucket named "media" and you'll need to do the same. We should make this more clear with a screenshot maybe

Thank you, that worked! Now I'm one step closer. Not sure if the problem was me or the documentation, probably me :D

cocobennett · 2021-04-09T20:37:17Z

Moving this to Ready for Review because I've added basic instructions for both in the UI and locally, as well as rebased correctly. I need help in step 2 of the local instructions as I myself don't know how to get from http://community.dev.readthedocs.io/ to my development version of the documentation. Thanks!

cocobennett · 2021-04-11T18:31:48Z

Hey there, I'd really like to help contribute here for myself and other newcomers. I'm trying to figure out how to find my development version of the docs after getting http://community.dev.readthedocs.io/ up and running.

I've tried clicking the "Documentation" link at the very bottom but this leads to me the actual live documentation

I've also tried guessing various urls based on the url generated from this PR itself, which is https://docs--8082.org.readthedocs.build/en/8082/. I've guessed the following urls without success to find my locally hosted documentation:

Any tips would be appreciated.

astrojuanlu · 2021-04-12T08:16:55Z

By the way, I opened #8090 to clarify the MinIO settings, I agree they're confusing.

astrojuanlu · 2021-04-12T09:55:11Z

I've tried clicking the "Documentation" link at the very bottom but this leads to me the actual live documentation

I wonder if the links on the footer should be relative instead of absolute.

I'm trying to figure out how to find my development version of the docs after getting http://community.dev.readthedocs.io/ up and running.

You'd need to follow some extra steps that, to my knowledge, are not documented yet:

Log in as admin/admin
Go to the "Read the Docs" project
Build latest documentation
Wait for it to finish and open http://read-the-docs.community.dev.readthedocs.io/en/latest/

In my opinion, these steps should be in https://docs.readthedocs.io/en/stable/development/install.html#set-up-your-environment, as a way to do an end-to-end test to check if everything worked. cc @readthedocs/core

humitos · 2021-04-12T10:44:15Z

In my opinion, these steps should be in docs.readthedocs.io/en/stable/development/install.html#set-up-your-environment, as a way to do an end-to-end test to check if everything worked. cc @readthedocs/core

Sounds good to me! I'd add a section after "Set up your environment" communicating this and making sure that everything is working as expected.

astrojuanlu

Left a couple of comments, but I think this is a step in the right direction!

docs/development/docs.rst

cocobennett · 2021-04-12T22:22:21Z

Thanks for the review and for the instructions @astrojuanlu ! I'm not sure which steps should be in "Setup your Environment" vs what steps should be listed in the file I'm editing here, so I've added them all to the current file for now. Maybe just the first step to ensure logging in as admin/admin works?

I ran into another issue though, both this link (http://read-the-docs.community.dev.readthedocs.io/en/latest/) and the "View Docs" button in the UI both lead me to this page instead of the documentation

docs/development/docs.rst

ericholscher · 2021-04-12T22:34:14Z

I ran into another issue though, both this link (http://read-the-docs.community.dev.readthedocs.io/en/latest/) and the "View Docs" button in the UI both lead me to this page instead of the documentation

I believe this is another Minio permissions issue. You need to do the same thing for the media bucket as you did for the static bucket: #8082 (comment)

docs/development/docs.rst

cocobennett · 2021-04-13T01:50:32Z

Done for the night. Hit a roadblock with a bunch of pip installs not found in the requirements/docs.txt. Not sure if it's a path issue on my end or something else. Will check again tomorrow

astrojuanlu · 2021-04-13T13:09:34Z

Indeed, requirements/docs.txt does not contain all the required dependencies. The command tox -e docs, which is very superficially mentioned in https://docs.readthedocs.io/en/stable/development/tests.html?highlight=docs#testing, works¹. Perhaps we should just bring it up in the contribution documentation as well?

This is essentially what `tox -e docs` does:

(.venv37) $ pip install -r requirements/testing.txt
(.venv37) $ pip install -r requirements/docs.txt
(.venv37) $ cd docs && make html

The docs.txt is mentioned in the tox configuration with a TODO:

readthedocs.org/tox.ini

Line 31 in db24ee1

    
           pip install -r{toxinidir}/requirements/docs.txt  # TODO: Remove this once our Sphinx versions don't conflict

and there was some previous discussion about what would be the best way to do this: #7037 (comment)

It would be interesting to analyze if we are already in a position to simplify this.

^{1 it's easier if run from Python 3.7, because orjson is pinned to 2.0.7, which doesn't have wheels for >= 3.8 and requires the Rust toolchain to compile}

cocobennett · 2021-04-13T21:36:10Z

Indeed, requirements/docs.txt does not contain all the required dependencies. The command tox -e docs, which is very superficially mentioned in https://docs.readthedocs.io/en/stable/development/tests.html?highlight=docs#testing, works1. Perhaps we should just bring it up in the contribution documentation as well?

1 it's easier if run from Python 3.7, because orjson is pinned to 2.0.7, which doesn't have wheels for >= 3.8 and requires the Rust toolchain to compile

EDIT: Just figured out the 3.7 python hint, and I'm running later than that. Please disregard below. I will retry tonight with python 3.7

Thanks! tox -e docs seems to get me past that step! But sorry guys, I'm running into another issue. Posting it here because I'll be away from the computer for a few hours.

In a clean venv, I ran tox -e docs and ran into an error about needing rust, so I followed the instructions given in the error code to this url. After that, I'm still running into errors. I tried this and this and still getting the following error and not sure how to proceed at the moment.

Getting requirements to build wheel: finished with status 'done'
    Preparing wheel metadata: started
    Preparing wheel metadata: finished with status 'error'
    ERROR: Command errored out with exit status 1:
     command: /home/coco/Projects/readthedocs.org/.tox/docs/bin/python /home/coco/Projects/readthedocs.org/.tox/docs/lib/python3.8/site-packages/pep517/_in_process.py prepare_metadata_for_build_wheel /tmp/tmps417n3va
         cwd: /tmp/pip-install-xsa2bpx3/orjson
    Complete output (29 lines):
    💥 pyo3-pack failed
      Caused by: Cargo metadata failed: Error during execution of `cargo metadata`:     Updating crates.io index
        Updating git repository `https://github.com/PyO3/pyo3.git`
        Updating git repository `https://github.com/ijl/json.git`
    error: failed to get `serde_json` as a dependency of package `orjson v2.0.7 (/tmp/pip-install-xsa2bpx3/orjson)`

    Caused by:
      failed to load source for dependency `serde_json`

    Caused by:
      Unable to update https://github.com/ijl/json.git?rev=2c0c55b628c9177543283a1535549e1dcb6a870a

    Caused by:
      failed to fetch into: /home/coco/.cargo/git/db/json-240b451de80eb693

    Caused by:
      failed to authenticate when downloading repository

      * attempted to find username/password via git's `credential.helper` support, but failed

      if the git CLI succeeds then `net.git-fetch-with-cli` may help here
      https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli

    Caused by:
      failed to acquire username/password from local configuration

    Checking for Rust toolchain....
    Running `pyo3-pack pep517 write-dist-info --metadata-directory /tmp/pip-modern-metadata-6xla9il6 --manylinux=off --rustc-extra-args=-C target-feature=+sse2 --strip=on`
    Error: Command '['pyo3-pack', 'pep517', 'write-dist-info', '--metadata-directory', '/tmp/pip-modern-metadata-6xla9il6', '--manylinux=off', '--rustc-extra-args=-C target-feature=+sse2', '--strip=on']' returned non-zero exit status 1.
    ----------------------------------------
ERROR: Command errored out with exit status 1: /home/coco/Projects/readthedocs.org/.tox/docs/bin/python /home/coco/Projects/readthedocs.org/.tox/docs/lib/python3.8/site-packages/pep517/_in_process.py prepare_metadata_for_build_wheel /tmp/tmps417n3va Check the logs for full command output.

=========================================================================== log end ===========================================================================ERROR: could not install deps [-r/home/coco/Projects/readthedocs.org/requirements/testing.txt]; v = InvocationError('/home/coco/Projects/readthedocs.org/.tox/docs/bin/python -m pip install -r/home/coco/Projects/readthedocs.org/requirements/testing.txt', 1)
___________________________________________________________________________ summary ___________________________________________________________________________ERROR:   docs: could not install deps [-r/home/coco/Projects/readthedocs.org/requirements/testing.txt]; v = InvocationError('/home/coco/Projects/readthedocs.org/.tox/docs/bin/python -m pip install -r/home/coco/Projects/readthedocs.org/requirements/testing.txt', 1)

cocobennett · 2021-04-14T17:27:59Z

it's easier if run from Python 3.7, because orjson is pinned to 2.0.7

Just in case, we are using Python 3.6.x in all our stack. So, I'd recommend to use that version that's well tested.

Even with a python3.6 environment, I ran tox -e docs then get an error to install rust, then I'm still seeing more errors.

Getting requirements to build wheel: finished with status 'done'
    Preparing wheel metadata: started
    Preparing wheel metadata: finished with status 'error'
    ERROR: Command errored out with exit status 1:
     command: /home/coco/Projects/readthedocs.org/.tox/docs/bin/python /home/coco/Projects/readthedocs.org/.tox/docs/lib/python3.8/site-packages/pep517/_in_process.py prepare_metadata_for_build_wheel /tmp/tmpiqmtppp0
         cwd: /tmp/pip-install-a4ti01ib/orjson
    Complete output (24 lines):
    💥 pyo3-pack failed
      Caused by: Cargo metadata failed: Error during execution of `cargo metadata`:     Updating crates.io index
    error: failed to get `encoding_rs` as a dependency of package `orjson v2.0.7 (/tmp/pip-install-a4ti01ib/orjson)`

    Caused by:
      failed to load source for dependency `encoding_rs`

    Caused by:
      Unable to update registry `https://github.com/rust-lang/crates.io-index`

    Caused by:
      failed to fetch `https://github.com/rust-lang/crates.io-index`

    Caused by:
      network failure seems to have happened
      if a proxy or similar is necessary `net.git-fetch-with-cli` may help here
      https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli

    Caused by:
      SSL error: error:1408F119:SSL routines:ssl3_get_record:decryption failed or bad record mac; class=Ssl (16)

    Checking for Rust toolchain....
    Running `pyo3-pack pep517 write-dist-info --metadata-directory /tmp/pip-modern-metadata-npry5xvm --manylinux=off --rustc-extra-args=-C target-feature=+sse2 --strip=on`
    Error: Command '['pyo3-pack', 'pep517', 'write-dist-info', '--metadata-directory', '/tmp/pip-modern-metadata-npry5xvm', '--manylinux=off', '--rustc-extra-args=-C target-feature=+sse2', '--strip=on']' returned non-zero exit status 1.
    ----------------------------------------
ERROR: Command errored out with exit status 1: /home/coco/Projects/readthedocs.org/.tox/docs/bin/python /home/coco/Projects/readthedocs.org/.tox/docs/lib/python3.8/site-packages/pep517/_in_process.py prepare_metadata_for_build_wheel /tmp/tmpiqmtppp0 Check the logs for full command output.

=========================================================================== log end ===========================================================================ERROR: could not install deps [-r/home/coco/Projects/readthedocs.org/requirements/testing.txt]; v = InvocationError('/home/coco/Projects/readthedocs.org/.tox/docs/bin/python -m pip install -r/home/coco/Projects/readthedocs.org/requirements/testing.txt', 1)
___________________________________________________________________________ summary ___________________________________________________________________________ERROR:   docs: could not install deps [-r/home/coco/Projects/readthedocs.org/requirements/testing.txt]; v = InvocationError('/home/coco/Projects/readthedocs.org/.tox/docs/bin/python -m pip install -r/home/coco/Projects/readthedocs.org/requirements/testing.txt', 1)

Maybe using the UI should be good enough if contributors don't setup the full stack?

astrojuanlu · 2021-04-14T19:07:03Z

Beware! For some reason, I think your tox -e docs is using a Python 3.8 interpreter (see for example the path /home/coco/Projects/readthedocs.org/.tox/docs/lib/python3.8/site-packages/pep517/_in_process.py

cocobennett · 2021-04-14T20:39:37Z

Thanks for the hint @astrojuanlu , that was the issue!!! I ran the pip commands separately since tox was using 3.8 and that worked for me :)

I'm ready for the final review now. Thanks for all the fast feedback!

cocobennett · 2021-04-14T20:41:10Z

requirements/testing.txt

@@ -17,7 +17,7 @@ execnet==1.8.0
 # supported platform.
 # mercurial-scm.org/wiki/SupportedPythonVersions
 # (Pinned to 4.4.2 since what we need for testing is still useful)
-Mercurial==4.4.2  # pyup: ignore
+# Mercurial==4.4.2  # pyup: ignore


For some reason, this kept failing, but everything else worked when I commented it out. I'm sure this is used elsewhere so I know this should be handled a better way

docs/development/docs.rst

stsewd

thanks! I left a suggestion to use sphinx-autobuild

docs/development/docs.rst

docs/img/details-link.png:Zone.Identifier

stsewd

Thanks for sticking with us! Sorry you had to setup the whole RTD instance with docker when it wasn't needed.

cocobennett · 2021-04-15T01:10:56Z

Thanks for sticking with us! Sorry you had to setup the whole RTD instance with docker when it wasn't needed.

No problem! I learned a lot. So happy to see this through :)

astrojuanlu

I think this is definitely an improvement, and I'm happy that it sparked a couple of useful conversations (#8090, #8108). Thanks a lot @cocobennett!

humitos · 2021-04-15T10:35:03Z

docs/development/docs.rst

+      pip install -r requirements/testing.txt
+      pip install -r requirements/docs.txt


Shouldn't we walk the user through the steps to create a virtualenv for this?

humitos · 2021-04-15T10:35:21Z

docs/development/docs.rst

+   .. prompt:: bash
+
+      cd readthedocs.org
+      pip install -r requirements/testing.txt


Do we need requirements/testing.txt to work on the docs?

At the moment they're needed, yes. Otherwise some dependencies are missing.

(It's up to debate whether we should organize our dependencies in a better way, or improve the naming of the requirements files, see #7037 (comment))

cocobennett changed the title ~~Improve docs~~ Improve documentation on contributing to documentation Apr 9, 2021

cocobennett added 2 commits April 9, 2021 15:20

add github UI instructions

681a3d3

add local instructions

524245c

cocobennett force-pushed the improve-docs branch from 7b03a74 to 524245c Compare April 9, 2021 20:34

remove periods to follow convention

c2df1c4

cocobennett marked this pull request as ready for review April 9, 2021 20:36

link style markdown -> rst

be428e5

astrojuanlu mentioned this pull request Apr 12, 2021

Clarify MinIO development settings #8090

Closed

astrojuanlu reviewed Apr 12, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

docs/development/docs.rst Outdated Show resolved Hide resolved

enhance local instructions

5111243

fix link syntax mistake

f87f74b

ericholscher reviewed Apr 12, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

stsewd reviewed Apr 12, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

docs/development/docs.rst Outdated Show resolved Hide resolved

cocobennett added 2 commits April 12, 2021 20:15

instructions without installing all of RTD

ca58bd3

numbers to #

9fee53f

working instructions for building documentation

94dc3e9

cocobennett commented Apr 14, 2021

View reviewed changes

cocobennett added 3 commits April 14, 2021 15:52

fix lint

bb037a4

fix file permissions

6b6a640

fix local setup issue with Mercurial

fe39794

cocobennett commented Apr 14, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

stsewd reviewed Apr 14, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

stsewd reviewed Apr 14, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

stsewd reviewed Apr 14, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

stsewd reviewed Apr 14, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

stsewd reviewed Apr 14, 2021

View reviewed changes

cocobennett added 6 commits April 14, 2021 18:30

remove tox instructions

71274a0

add image

b4a8692

move todo comment

94a1e40

python3.7->3.6

780f639

install autobuild and update instructions

6fe163a

autobuild

2b5024a

stsewd reviewed Apr 15, 2021

View reviewed changes

docs/development/docs.rst Outdated Show resolved Hide resolved

docs/img/details-link.png:Zone.Identifier Outdated Show resolved Hide resolved

review edits

5f477e4

stsewd approved these changes Apr 15, 2021

View reviewed changes

astrojuanlu mentioned this pull request Apr 15, 2021

Docs: Adopt a style guide #8108

Open

astrojuanlu approved these changes Apr 15, 2021

View reviewed changes

astrojuanlu merged commit a4ed8d9 into readthedocs:master Apr 15, 2021
3 checks passed

humitos reviewed Apr 15, 2021

View reviewed changes

cocobennett deleted the improve-docs branch April 15, 2021 16:07

Improve documentation on contributing to documentation #8082

Improve documentation on contributing to documentation #8082

cocobennett commented Apr 9, 2021

astrojuanlu commented Apr 9, 2021

cocobennett commented Apr 9, 2021

cocobennett commented Apr 9, 2021

stsewd commented Apr 9, 2021 •

edited

cocobennett commented Apr 9, 2021

cocobennett commented Apr 9, 2021

cocobennett commented Apr 11, 2021

astrojuanlu commented Apr 12, 2021

astrojuanlu commented Apr 12, 2021

humitos commented Apr 12, 2021

astrojuanlu left a comment

cocobennett commented Apr 12, 2021

ericholscher commented Apr 12, 2021

cocobennett commented Apr 13, 2021

astrojuanlu commented Apr 13, 2021

cocobennett commented Apr 13, 2021 •

edited

cocobennett commented Apr 14, 2021

astrojuanlu commented Apr 14, 2021

cocobennett commented Apr 14, 2021

cocobennett Apr 14, 2021

stsewd left a comment

stsewd left a comment

cocobennett commented Apr 15, 2021

astrojuanlu left a comment

humitos Apr 15, 2021

humitos Apr 15, 2021

astrojuanlu Apr 15, 2021

astrojuanlu Apr 15, 2021

		pip install -r requirements/testing.txt
		pip install -r requirements/docs.txt

Improve documentation on contributing to documentation #8082

Improve documentation on contributing to documentation #8082

Conversation

cocobennett commented Apr 9, 2021

astrojuanlu commented Apr 9, 2021

cocobennett commented Apr 9, 2021

cocobennett commented Apr 9, 2021

stsewd commented Apr 9, 2021 • edited

cocobennett commented Apr 9, 2021

cocobennett commented Apr 9, 2021

cocobennett commented Apr 11, 2021

astrojuanlu commented Apr 12, 2021

astrojuanlu commented Apr 12, 2021

humitos commented Apr 12, 2021

astrojuanlu left a comment

Choose a reason for hiding this comment

cocobennett commented Apr 12, 2021

ericholscher commented Apr 12, 2021

cocobennett commented Apr 13, 2021

astrojuanlu commented Apr 13, 2021

cocobennett commented Apr 13, 2021 • edited

cocobennett commented Apr 14, 2021

astrojuanlu commented Apr 14, 2021

cocobennett commented Apr 14, 2021

cocobennett Apr 14, 2021

Choose a reason for hiding this comment

stsewd left a comment

Choose a reason for hiding this comment

stsewd left a comment

Choose a reason for hiding this comment

cocobennett commented Apr 15, 2021

astrojuanlu left a comment

Choose a reason for hiding this comment

humitos Apr 15, 2021

Choose a reason for hiding this comment

humitos Apr 15, 2021

Choose a reason for hiding this comment

astrojuanlu Apr 15, 2021

Choose a reason for hiding this comment

astrojuanlu Apr 15, 2021

Choose a reason for hiding this comment

stsewd commented Apr 9, 2021 •

edited

cocobennett commented Apr 13, 2021 •

edited