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

Run the build weekly, against the latest Sphinx version #70

Merged
merged 2 commits into from
Apr 25, 2021

Conversation

pradyunsg
Copy link
Collaborator

I just noticed that #69 was made and merged without reviews. Here's my attempt to solve the underlying issues in a better way.

@pradyunsg pradyunsg requested a review from shirou April 18, 2021 15:05
@pradyunsg pradyunsg changed the title Run the build regularly, against the latest Sphinx version Run the build weekly, against the latest Sphinx version Apr 18, 2021
Copy link
Contributor

@shirou shirou left a comment

Choose a reason for hiding this comment

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

Thank you. it looks great!

When I have tested, Sphinx 4.0.0b1 is not installed without version specific even I add --pre. so I think version is required, but now I tested again and confirmed no need to specify.

@shirou
Copy link
Contributor

shirou commented Apr 19, 2021

But, hmm, it seems pydata-sphinx-theme has been updated 2 days ago and perhaps because of that update, 4.0.0b1 is failing.

@pradyunsg
Copy link
Collaborator Author

pradyunsg commented Apr 19, 2021

@choldgraf @jorisvandenbossche FYI: the latest pydata-sphinx-theme release is failing to build with the latest Sphinx release.

@choldgraf
Copy link
Contributor

This is the failing line https://github.com/sphinx-themes/sphinx-themes.org/pull/70/checks?check_run_id=2377148747#step:8:152

@jorisvandenbossche
Copy link

Strange, I don't see a failure locally with our demo docs using sphinx 4.0.0 beta.

@pradyunsg
Copy link
Collaborator Author

Rebasing this on master, which has a trimmed list of themes now (#72); and also removed pydata-sphinx-theme from the list to unblock this.

We should implement something to prevent broken themes from breaking the build process in the future.

Comment on lines +30 to +31
env.install("--pre", f"sphinx") # we test against the latest Sphinx version
env.install(theme.pypi)
Copy link
Collaborator Author

@pradyunsg pradyunsg Apr 25, 2021

Choose a reason for hiding this comment

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

Note that this means that if the theme specifies its dependencies like sphinx<4, it wouldn't be built against the latest pre-release, but that's fine since the theme has an explicit constraint.

@pradyunsg pradyunsg merged commit d06e64a into sphinx-themes:master Apr 25, 2021
@pradyunsg pradyunsg deleted the build-regularly branch April 25, 2021 14:57
@choldgraf
Copy link
Contributor

Does this mean that we need to remember to add the pydata theme to this page's preview in the future?

And more generally - it sounds like this PR is implicitly implementing a policy for this repository that any theme that doesn't build on the next pre-release of Sphinx is removed from the website. Is that true? If so we should document this.

@pradyunsg
Copy link
Collaborator Author

As a broader policy, I do think this site will be building against the latest Sphinx release (including the latest pre-release) that the theme's latest release claims to be compatible with. I'll document that in a follow up PR sometime next week.

I'll also be adding the pydata-sphinx-theme back, once a release containing a fix happens or I come around to resolving the whole the entire-build-fails-because-one-theme-failed behaviour -- whichever is first. I don't want to be removing-readding themes constantly though, especially with a weekly build cycle, but that can't change until we have more graceful handling of failures here.

@choldgraf
Copy link
Contributor

Sounds good, I agree w/ all of that :-) I just wanted to make sure we were on the same page!

@jorisvandenbossche
Copy link

BTW I released pydata-sphinx-theme so it should no longer fail with latest sphinx/jinja2

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

4 participants