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 readthedocs config to version 2 #1674

Merged
merged 5 commits into from
Aug 13, 2024
Merged

Update readthedocs config to version 2 #1674

merged 5 commits into from
Aug 13, 2024

Conversation

lucc
Copy link
Collaborator

@lucc lucc commented Aug 9, 2024

The docs for the new config file format are here: https://docs.readthedocs.io/en/stable/config-file/

I am not sure if that whole build will succeed because we might need notmuch or gpg. In their docs they also advertise a requirements.txt again :)

@pazz can you test this branch on RTD?

@lucc lucc added this to the release 0.11 milestone Aug 9, 2024
@pazz
Copy link
Owner

pazz commented Aug 9, 2024

Seems to fail as it tries to install notmuch2 via pip.

rtd.txt

@lucc
Copy link
Collaborator Author

lucc commented Aug 9, 2024

Thanks for activating that branch. The logs are public so I can debug this myself.

This needs python 3.11 or higher to build the docs because tomllib is
only available from python 3.11 on.
@lucc lucc requested a review from pazz August 10, 2024 07:13
@lucc
Copy link
Collaborator Author

lucc commented Aug 10, 2024

This now builds on RTD and is ready for review.

I found out that we can have RTD build pull requests. They can even report the build status back to Github which might save us from broken docs after a PR. See https://docs.readthedocs.io/en/stable/guides/pull-requests.html, the status stuff is my interpretation of https://docs.readthedocs.io/en/stable/guides/pull-requests.html#troubleshooting .

open questions @pazz

  • Do you know what the file second config file in docs/source/api/conf.py is for? The file docs/source/index.rst links to api/index so it does not seem that we need to do a second build.

  • We now need python >= 3.11 to build the docs because I used tomllib from the stdlib in conf.py. Is that a problem for us? What is stopping us from just lifting the python requirement for the whole of alot?
    I remember our discussion in one of the dependabot issues. I would still propose the same argument: If we want to use a higher version constraint for some reason we should do it. Any downstream packager who can not follow the version bump needs to package an old version of alot. (I know that you are interested in Debian: python 3.11 is available with debian 12 ;-)

PS: But we could also keep the python version requirements as is because we only need py3.11 to run sphinx to generate the docs.

@pazz
Copy link
Owner

pazz commented Aug 12, 2024

open questions @pazz

  • Do you know what the file second config file in docs/source/api/conf.py is for? The file docs/source/index.rst links to api/index so it does not seem that we need to do a second build.

Pretty sure this was checked in by accident. It looks like it is unused and outdated.

  • We now need python >= 3.11 to build the docs because I used tomllib from the stdlib in conf.py. Is that a problem for us? What is stopping us from just lifting the python requirement for the whole of alot?

I do not have an issue with upping the version dependency for alot to 3.11.

Copy link
Owner

@pazz pazz left a comment

Choose a reason for hiding this comment

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

It may be a good idea to get the metatata from importlib (requires >=v3.8 IIRC) rather than tomlib by hand.

The format of the intersphinx entries should also be updated now that we're at it, as that causes warnings for me locally (see my linked patch).

Lastly, you may want to add your email to the metadata, rather than just your name :)

docs/source/api/conf.py Show resolved Hide resolved
@lucc
Copy link
Collaborator Author

lucc commented Aug 12, 2024

Does the dynamic version branch build on rtd? I had the impression that the importlib.version function needs a working installation of the package it tries to find. That would force us to install alot with notmuch and gpg on rtd! I would prefer to read the static data from the toml file instead of having pip read it from the toml file and then importlib fetch it from some pip generated metadata again.

@pazz
Copy link
Owner

pazz commented Aug 12, 2024 via email

@lucc
Copy link
Collaborator Author

lucc commented Aug 12, 2024

@pazz I do not understad your last comment. This branch (rtd) contains a change to the intersphinx settings in d6aedec . What more is needed?

(I welcome a dynamic version, I am using setuptools_scm in khard for example)

@pazz
Copy link
Owner

pazz commented Aug 13, 2024

@pazz I do not understad your last comment. This branch (rtd) contains a change to the intersphinx settings in d6aedec . What more is needed?

(I welcome a dynamic version, I am using setuptools_scm in khard for example)

I overlooked this, sorry. LGTM then :)

@pazz pazz merged commit 1c49fa5 into master Aug 13, 2024
10 checks passed
@lucc lucc deleted the rtd branch August 13, 2024 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants