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

Add initial Sphinx docs skeleton #1429

Merged
merged 13 commits into from
Aug 12, 2021
Merged

Add initial Sphinx docs skeleton #1429

merged 13 commits into from
Aug 12, 2021

Conversation

l1storez
Copy link
Contributor

@l1storez l1storez commented Jun 21, 2021

The only doc currently is the project README. It's grown over time and now it may be a good idea to split that information into pieces

I decided to add a simple docs skeleton using sphinx

Implemented steps:

  1. Created a docs/ dir.
  2. Generated a docs project skeleton, the RST documents are moved right into docs/
  3. Dropped any makefile mentions
  4. Added a tox env for building docs
  5. Edited index.rst to include the current README
  6. Edited docs/conf.py to enable nitpicky mode and make sure to add -W --keep-going to the Sphinx invocations in tox
  7. Integrated MyST extension
  8. Integrated Furo theme.
  9. Specified all the deps in docs/requirements.txt
  10. Added a separate sphinx invocation with -b linkcheck
  11. Added those tox envs to GitHub Actions CI/CD workflows.
  12. Added .readthedocs.yml to configure Read The Docs. Make sure to set htmldir builder and fail on warnings.
Contributor checklist
  • Provided the tests for the changes.
  • Assure PR title is short, clear, and good to be included in the user-oriented changelog
  • Assure one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

Fixes: #1427

@ssbarnea ssbarnea added the enhancement Improvements to functionality label Jun 22, 2021
@ssbarnea ssbarnea changed the title 1427. Adding an initial Sphinx docs skeleton Add initial Sphinx docs skeleton Jun 22, 2021
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Don't move README. It must remain in the project root.

docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@l1storez
Copy link
Contributor Author

l1storez commented Jul 7, 2021

okay

tox.ini Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.readthedocs.yml Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.readthedocs.yml Outdated Show resolved Hide resolved
.readthedocs.yml Outdated Show resolved Hide resolved
.readthedocs.yml Outdated Show resolved Hide resolved
.readthedocs.yml Outdated Show resolved Hide resolved
docs/index.rst Show resolved Hide resolved
docs/requirements.txt Outdated Show resolved Hide resolved
@webknjaz
Copy link
Member

webknjaz commented Aug 8, 2021

Hey @jezdez, could you please enable RTD and PR builds for this repo?

docs/conf.py Show resolved Hide resolved
.readthedocs.yml Outdated Show resolved Hide resolved
.readthedocs.yml Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
docs/requirements.txt Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
@l1storez
Copy link
Contributor Author

Squash commit messages and add .gitinore in the docs/ dir

.readthedocs.yml Outdated Show resolved Hide resolved
docs/.gitignore Outdated Show resolved Hide resolved
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Do not move README.rst under docs/. Change it back.


project = "pip-tools"
author = "Jazzband Contributors"
copyright = f"2021, {author}"
Copy link
Member

Choose a reason for hiding this comment

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

@jezdez do we have any policy on the copyright format? Technically this should be the year of creation (2012?). But then, some communities remove it altogether because it's not a well-maintained number.

Copy link
Member

Choose a reason for hiding this comment

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

No Jazzband specific policy, but the first year of creation is sensible to indicate when the project was started. It shouldn't be "Jazzband Contributors" but "pip-tools contributors" as well.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, okay. According to my prior research, the copyright year of the content creation should be stated and then, the years of the updates to the content (list or range). Using the current year (and updating it every January) is incorrect because this reflects the invalid info.

OTOH my understanding is that the copyright notice by itself doesn't have any legal power — it's mostly informational and the copyright is assigned to the (parts) of the content regardless of what that string says.

But because many people interpret it differently, such a statement tends to be incorrect and/or out-of-date. This is why the recent recommendations that I saw were not to use it at all (for the ease of maintenance). I think somebody from RH legal noticed this approach and also the Linux Foundation recommends it: https://www.linuxfoundation.org/blog/copyright-notices-in-open-source-software-projects/.

P.S. Of course, IANAL so takes this with a grain of salt. Still, I do like the recommendation not to state the year in the copyright lines.


As for "community contributors" vs "project contributors", I saw both. But I don't have a problem with just accepting what you prefer. Just wanted to put this comment in the public, for future reference.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, okay. According to my prior research, the copyright year of the content creation should be stated and then, the years of the updates to the content (list or range). Using the current year (and updating it every January) is incorrect because this reflects the invalid info.

OTOH my understanding is that the copyright notice by itself doesn't have any legal power — it's mostly informational and the copyright is assigned to the (parts) of the content regardless of what that string says.

But because many people interpret it differently, such a statement tends to be incorrect and/or out-of-date. This is why the recent recommendations that I saw were not to use it at all (for the ease of maintenance). I think somebody from RH legal noticed this approach and also the Linux Foundation recommends it: https://www.linuxfoundation.org/blog/copyright-notices-in-open-source-software-projects/.

P.S. Of course, IANAL so takes this with a grain of salt. Still, I do like the recommendation not to state the year in the copyright lines.

Sure that works for me, let's skip the year number, the changelog provides ample reference to the updates anyway.

As for "community contributors" vs "project contributors", I saw both. But I don't have a problem with just accepting what you prefer. Just wanted to put this comment in the public, for future reference.

I'd prefer to point out that not all community members are automatically contributors to the project. And pip-tools has had contributors before it became a community project. So it's easiest to refer to the contributors as "pip-tools contributors" IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. This will be addressed via #1467.

@l1storez
Copy link
Contributor Author

Do not move README.rst under docs/. Change it back.

Probably, it happened after a not good rebase

l1storez and others added 3 commits August 11, 2021 23:15
Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

I think that this is ready for merge, as an initial effort. It's best to add any further improvements in follow-up PRs.

@webknjaz webknjaz marked this pull request as ready for review August 12, 2021 11:27
@l1storez
Copy link
Contributor Author

Oh, wow. PR is alive

@webknjaz webknjaz added docs Documentation related skip-changelog Avoid listing in changelog labels Aug 12, 2021
@webknjaz webknjaz merged commit 636e7f0 into jazzband:master Aug 12, 2021
@jezdez
Copy link
Member

jezdez commented Aug 12, 2021

Hey @jezdez, could you please enable RTD and PR builds for this repo?

Please provide access to the "jazzband-bot" user on Read The Docs.

@jezdez
Copy link
Member

jezdez commented Aug 12, 2021

Hey @jezdez, could you please enable RTD and PR builds for this repo?

Please provide access to the "jazzband-bot" user on Read The Docs.

Nevermind, saw that you added me personally to the RTD project.

@webknjaz
Copy link
Member

Yep, was about to add the bot too :)

@jezdez
Copy link
Member

jezdez commented Aug 12, 2021

The docs build fails because the Sphinx conf file doesn't have a version. I would suggest to use sphinx-quickstart to create a conf.py with all the variables populated.

@webknjaz
Copy link
Member

The docs build fails because the Sphinx conf file doesn't have a version. I would suggest to use sphinx-quickstart to create a conf.py with all the variables populated.

Yeah, I saw that too. I guess I overlooked that. It's actually failing because the warnings are turned into errors in the nitpicky mode (although, I don't quite understand why RTD is failing but GHA isn't). I'll look into it and correct what's missing.

P.S. I also saw some image-related warnings on RTD too. I wonder if it's some Sphinx ext that is different on RTD and gets populated in that env or it's just flaky...

@webknjaz
Copy link
Member

FWIW I'm happy that we got this merged at least in this state because it gives up many possibilities to improve things. And maybe even a bunch of easy new-contributor issues could come out of this, which is always good for letting new folks get used to the project setup :)

@l1storez
Copy link
Contributor Author

if I get you right, after removing "unnecessary lines" and rebasing commits the docs build fails?

@webknjaz
Copy link
Member

Building HTML doesn't fail (-b html or -b htmldir) but building EPUB3 fails (-b epub). Since we don't do this in the CI, we never noticed it. Now that we've enabled RTD, it attempted to build the site and failed. This is because we request it to also build additional formats like EPUB and PDF.
Here's the log: https://readthedocs.org/projects/pip-tools/builds/14458980/.
But don't worry, I'll address that shortly.

@webknjaz
Copy link
Member

This should fix it: #1466.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related enhancement Improvements to functionality skip-changelog Avoid listing in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TODO] Add an initial Sphinx docs skeleton
4 participants