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

Developer docs emphasize the Docker setup #6682

Merged
merged 9 commits into from Apr 6, 2020

Conversation

davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Feb 19, 2020

If we are happy with the RTD Docker development setup, we should emphasize them over setting up a local install. These changes emphasize those docs and have other docs and guides point to the Docker development setup:

  • Make the docker compose setup guide the first developer doc.
  • Deprioritize the "install" doc which perhaps should be removed.
  • Note that the search setup is part of the docker compose setup guide
  • Remove section on pre-commit (not all the core team is using this). Get developers right to the setup process!
  • Add an index to /development so it doesn't 404 (eg. https://docs.readthedocs.io/en/stable/development/)

- Remove section on pre-commit (not all the core team is using this)
- Make the docker compose setup guide the first developer doc.
- Deprioritize the "install" doc which perhaps should be removed.
- Note that the search setup is part of the docker compose setup guide
@davidfischer davidfischer requested a review from Feb 19, 2020
Copy link
Member

@humitos humitos left a comment

I'm 👍 on this. I think docker compose has been stable enough to start recommending it as the common setup for contributors at this point.

docs/development/install.rst Outdated Show resolved Hide resolved
docs/development/search.rst Outdated Show resolved Hide resolved
paths. Before testing, make sure you have Tox installed::
paths. Before testing, make sure you have Tox installed:

.. prompt:: bash

pip install tox
Copy link
Member

@humitos humitos Feb 20, 2020

Choose a reason for hiding this comment

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

We are running tests from inside docker compose setup now. It may be better to mention that here instead.

inv docker.test
inv docker.test -a '-e py36'

Copy link
Contributor Author

@davidfischer davidfischer Feb 20, 2020

Choose a reason for hiding this comment

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

I can't recommend that yet, I think. On my setup this takes a really long time to run.

@davidfischer
Copy link
Contributor Author

@davidfischer davidfischer commented Apr 2, 2020

I think this is ready to merge if there are no objections.

Copy link
Member

@ericholscher ericholscher left a comment

I think this is an OK change, but it still feels too hidden to me. I think we should link this content from the front-page of our docs at least,

@@ -116,7 +71,6 @@ for your own :doc:`custom installation <custom_installs/index>`.
.. toctree::
:maxdepth: 1

development/install
development/standards
Copy link
Member

@ericholscher ericholscher Apr 6, 2020

Choose a reason for hiding this comment

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

I still don't quite understand how I'm meant to get to this TOC. It's hidden in a second-level page with the title "Contributing" and then not even in view when you load the page. I'd really like to at least get some of this content onto the main TOC and front page of the docs.

Currently searching for "install" or "develop" doesn't lead to this content when hitting the front-page of the site, which I think we should likely address here.

Copy link
Member

@ericholscher ericholscher Apr 6, 2020

Choose a reason for hiding this comment

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

Of note, this is also a weird place for this TOC -- it should live in the development/index.rst, not inside a contribute.rst at the top-level. I think linking to that index and adding it to the top-level TOC would solve a lot of my issue here.

Copy link
Member

@ericholscher ericholscher Apr 6, 2020

Choose a reason for hiding this comment

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

I am putting together a PR with my suggestions.

Copy link
Member

@ericholscher ericholscher Apr 6, 2020

Choose a reason for hiding this comment

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

ericholscher and others added 4 commits Apr 6, 2020
Copy link
Member

@ericholscher ericholscher left a comment

🎉

@ericholscher ericholscher merged commit b72343f into master Apr 6, 2020
2 checks passed
@ericholscher ericholscher deleted the davidfischer/streamline-developer-docs branch Apr 6, 2020
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