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

Adjust to RTD's upcoming addon changes #12

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Conversation

glatterf42
Copy link
Member

@glatterf42 glatterf42 commented Jul 23, 2024

Starting on October 7, RTD will switch to RTD Addons for sphinx projects, too. They have identified that this is going to impact message_ix and suggest to change our configuration file in almost exactly the way this PR does. I have enabled Addons temporarily on RTD and intend to use this PR to make any adjustments necessary to keep our docs building as expected.

How to review

  • Read the diff and note that the CI checks all pass.
  • Look at the docs built from this PR and confirm they are what we want/expect.

PR checklist

  • Continuous integration checks all ✅
  • Add, expand, or update documentation.

@glatterf42 glatterf42 added the documentation Improvements or additions to documentation label Jul 23, 2024
@glatterf42 glatterf42 self-assigned this Jul 23, 2024
@glatterf42
Copy link
Member Author

This project didn't have the setting to build a version of this PR's docs on RTD, which I changed for the moment. I'll try closing and re-opening this PR to see if this triggers the build process.

@glatterf42 glatterf42 closed this Jul 23, 2024
@glatterf42 glatterf42 reopened this Jul 23, 2024
@glatterf42
Copy link
Member Author

glatterf42 commented Jul 23, 2024

It does 🎉

This raises the question, though: what does the GitHub CI give us that the RTD check doesn't provide? A faster runtime, for sure, but RTD then also compiles the pages so that we can look at them: https://iiasa-energy-program-ece-docs--12.com.readthedocs.build/scenario-databases.html

Please note that visual changes like the flyout menu switching to the right side are expected (see migration notes). RTD wants to update their theme to offer keeping it on the left, but for now, this change is intentional.

With that in mind, the docs look great to me and don't produce any error message :)

@danielhuppmann
Copy link
Member

I didn't know that there is an RTD-workflow - first time I see this... But super-cool, and indeed, ok to remove the GitHub workflow if RTD has a native solution.

I'm wondering if this will also happen on pyam, message_ix, etc.? And if this only works on PRs from the upstream repository or also from forks?

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Suggest to remove the GitHub Actions workflow if this is not necessary any more

@glatterf42
Copy link
Member Author

The workflow installs the package via pip -r requirements.txt and then builds the docs via make html. RTD does the same, except for calling sphinx html in the end, which seems fine to me.

RTD sent out an email about message_ix, ece-docs, and ece-teaching being affected. I have taken care of all of them. For pyam, it's not listed as a project under https://readthedocs.com/organizations/iiasa-energy-program/, so I did not get notified. If its configuration is similar to the one here or on message_ix, the migration is not difficult, though.

The RTD build process is triggered for PRs from forks, too, and it doesn't rely on GitHub secrets or so, so it's usually successful on message_ix :)

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Thanks @glatterf42!

@glatterf42 glatterf42 merged commit 03daab5 into main Jul 24, 2024
1 check passed
@glatterf42 glatterf42 deleted the enh/rtd-addon-compatibility branch July 24, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants