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

Update Contributing Docs #1915

Merged
merged 4 commits into from Jan 13, 2021
Merged

Update Contributing Docs #1915

merged 4 commits into from Jan 13, 2021

Conversation

cooperlees
Copy link
Collaborator

  • Update docs with all new tox hotness
  • Test running docs build:
    • sphinx-build -a -b html -W docs/ docs/_build/

Fixes #1907

- Update docs with all new tox hotness
- Test running docs build:
  - `sphinx-build -a -b html -W docs/ docs/_build/`

Fixes #1907
CONTRIBUTING.md Outdated
If you make changes to docs, you can test they still build locally too.

```console
$ pip install docs/requirements.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$ pip install docs/requirements.txt
$ pip install -r docs/requirements.txt

(haven't tested)

CONTRIBUTING.md Outdated
@@ -52,7 +77,7 @@ your PR. You may need to change
configuration for it to pass.

For more `black-primer` information visit the
[documentation](https://github.com/psf/black/blob/master/docs/black_primer.md#black-primer).
[documentation](https://github.com/psf/black/blob/master/docs/black_primer.md#).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
[documentation](https://github.com/psf/black/blob/master/docs/black_primer.md#).
[documentation](https://github.com/psf/black/blob/master/docs/black_primer.md).

Copy link
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

I'm not happy with the current state of the documentation workflow and the myriad of different ways of setting up a dev environment and then running the tests, but that's beyond the scope of this PR. Beyond that LGTM.

$ black-primer [-k -w /tmp/black_test_repos]
```

### Docs Testing

If you make changes to docs, you can test they still build locally too.
Copy link
Collaborator

@ichard26 ichard26 Jan 11, 2021

Choose a reason for hiding this comment

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

Technically this step is also needed when you make modifications to documentation files which are duplicated (for reasons I don't really know / understand). But it really doesn't matter if the duplicated content is up to date since they will updated before it get deploys on ReadTheDocs as a fresh build always starts, and IMO GitHub isn't the right place for documentation hosting. IIRC one of the reasons why we duplicate the docs is because it improves the GitHub documentation reading experience, but we have ReadTheDocs already, a platform designed for documentation. I honestly wish and might try to get these duplicated files and the regen step gone for good.

edit: this ended up being a lot more ranty than I expected (the words in bold are what matter), apologies for the wall of text!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I +1 this cleanup. I think it's not worth having the duplicates too.

I'm just keeping my PRs to address/solve one main problem with each, as you noted.

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.

Update contributing
3 participants