Skip to content
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 Read the Docs tutorial, part I #8428

Merged
merged 17 commits into from
Aug 24, 2021
Merged

New Read the Docs tutorial, part I #8428

merged 17 commits into from
Aug 24, 2021

Conversation

astrojuanlu
Copy link
Contributor

@astrojuanlu astrojuanlu commented Aug 18, 2021

This is the first pull request (hopefully out of many more to come) for the new Read the Docs tutorial proposed in #8106 in the context of the CZI grant. Paraphrasing @evildmp, it is by no means finished, but almost complete 🙂

Rendered version: https://docs--8428.org.readthedocs.build/en/8428/tutorial/

This pull request intends to cover chapter 2 of the table of contents originally proposed in #8106:

  1. Getting started
    1. Preparing our project on GitHub
    2. Importing our project to Read the Docs
    3. Basic configuration changes

In light of the recent conversations around #8410, I have added some detailed information about the signup process, since the tutorial is aimed at newcomers and we don't assume advanced GitHub knowledge.

The template mentioned in the tutorial is this one, at the moment living on my personal GitHub account before it is accepted:

https://github.com/astrojuanlu/tutorial-template/

@astrojuanlu astrojuanlu changed the title DRAFT: New Read the Docs tutorial, part I New Read the Docs tutorial, part I Aug 18, 2021
@astrojuanlu astrojuanlu marked this pull request as ready for review August 18, 2021 21:48
@astrojuanlu astrojuanlu requested review from a team and nienn August 18, 2021 21:48
@astrojuanlu
Copy link
Contributor Author

This is ready for review. Pinging @readthedocs/advocacy and also @nienn, in case she can offer a (much needed!) newcomer perspective and look at the tutorial with fresh eyes.

About your comment on #8410 (comment):

what I most missed was a brief introduction, without going into too much technical detail, about what is Read the Docs App, what problems is it trying to solve, what are it's dependencies and a clear, very much simplified, path that I would need to follow in order to get from where I was to having a complete working documentation project.

I haven't included such explanation yet (that would be part of section 1 "The Read the Docs way" as per our original proposal) but hopefully I can do it soon (perhaps in a future pull request).

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wonderful. 🎉 🎉

Lots of little nitpicks, but overall I think it's great and I'm super stoked to have it. ❤️

You will fork a fictional software library
similar to the one developed in the :doc:`official Sphinx tutorial <sphinx:tutorial/index>`.
No prior experience with Sphinx is required,
and you can follow this tutorial without having done the Sphinx one.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads nicely, and gives the user somewhere to learn more if needed. Love it 💯

docs/tutorial/index.rst Outdated Show resolved Hide resolved
docs/tutorial/index.rst Outdated Show resolved Hide resolved
docs/tutorial/index.rst Outdated Show resolved Hide resolved
docs/tutorial/index.rst Show resolved Hide resolved
Comment on lines 206 to 207
You can now proceed to make some basic configuration adjustments.
For that, click on the "⚙ Admin" button, which will open the Settings page.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to tell them that this is on the project page.

We should also clarify our language, is it Admin or Settings?

Suggested change
You can now proceed to make some basic configuration adjustments.
For that, click on the "⚙ Admin" button, which will open the Settings page.
You can now proceed to make some basic configuration adjustments.
Navigate back to the *project page* click on the "⚙ Admin" button, which will open the Settings page.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, will clarify

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also clarify our language, is it Admin or Settings?

Admin has a Settings section, as well as Advanced Settings, Edit Versions... but there is no "Admin page" per se. Not sure how to reword this.

To do so, click on the "Notifications" link on the left,
type the email where you would like to get the notification,
and click the "Add" button.
After that, your email will be shown under "Existing Notifications".
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UX on this page is so bad :( Really need to improve it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it's ugly 😓


To enable that functionality, first click on the "Advanced Settings" link on the left
under the "⚙ Admin" menu, check the "Build pull requests for this project" checkbox,
and click the "Save" button at the bottom of the page.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should definitely make this easier to do :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or evaluate the cost of making it opt-out? I hypothesize that most projects use only the default branch anyway (no extra $ on our side), and those who do use pull requests would really appreciate having this feature but they might not know about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I could be convinced, tho I worry it's the same as pdf/epub, where if it's on by default most users still won't use it (and might get annoyed by it). I think we need to promote it more, either way.

docs/tutorial/index.rst Outdated Show resolved Hide resolved
docs/tutorial/index.rst Show resolved Hide resolved
@astrojuanlu
Copy link
Contributor Author

Thanks @ericholscher for the super thorough review! Addressed the comments.

@astrojuanlu
Copy link
Contributor Author

Hmm the checks are failing with this error:

docs-lint run-test: commands[0] | rstcheck -r .
./tutorial/index.rst:124: (ERROR/3) Undefined substitution referenced: ":arrows_counterclockwise:".

however, the emoji renders correctly. Any idea of what might be happening here?

@stsewd
Copy link
Member

stsewd commented Aug 19, 2021

@astrojuanlu
Copy link
Contributor Author

Thanks @stsewd! Basically all emojis then... I added a few more to make this PR pass.

@ericholscher
Copy link
Member

@astrojuanlu you need to update this file https://github.com/readthedocs/readthedocs.org/blob/master/docs/.rstcheck.cfg

That seems silly :(

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@@ -1,7 +1,7 @@
[rstcheck]
ignore_directives=automodule,http:get,tabs,tab,prompt,http:patch,http:post,http:put,http:delete
ignore_roles=djangosetting,setting,featureflags,buildpyversions
ignore_substitutions=org_brand,com_brand,:smile:
ignore_substitutions=org_brand,com_brand,:smile:,:arrows_counterclockwise:,:heavy_plus_sign:,:tada:,:heart:,:pencil2:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice if we could just say ignore :*:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That won't work apparently https://github.com/myint/rstcheck/blob/3f92957478422df87bd730abde66f089cc1ee19b/rstcheck.py#L265-L266 and rstcheck seems to be unmaintained.

docs/glossary.rst Show resolved Hide resolved
docs/glossary.rst Show resolved Hide resolved
docs/tutorial/index.rst Outdated Show resolved Hide resolved
docs/tutorial/index.rst Show resolved Hide resolved
Basic configuration changes
~~~~~~~~~~~~~~~~~~~~~~~~~~~

You can now proceed to make some basic configuration adjustments.
For that, click on the "⚙ Admin" button, which will open the Settings page.
Navigate back to the :term:`project page`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@@ -268,3 +281,6 @@ If you click on the "Details" link while it is building,
you will access the build logs,
otherwise it will take you directly to the documentation.
When you are satisfied, you can merge the pull request!

That's the end of the tutorial for now,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now feels weird, but I don't have a great suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just remove it?

docs/index.rst Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants