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

Fix release of Sphinx by forcing upgrades to dependencies #9439

Closed
wants to merge 2 commits into from

Conversation

ericholscher
Copy link
Contributor

@ericholscher ericholscher commented Jul 12, 2021

Feature or Bugfix

  • Bugfix

Purpose

This fixes #9434, and should probably go into a new 4.1.1 release

Detail

We should probably have a proper versioning policy for these dependencies. Are they really all required? Should some be optional, and not break Sphinx if they aren't able to be imported?

It seems that each should probably be updated as a dependency on each Sphinx release, given that they cause breaking changes Sphinx if they get out of date.

Relates

#9434

@tk0miya tk0miya added this to the 4.1.1 milestone Jul 13, 2021
@tk0miya tk0miya changed the base branch from 4.x to 4.1.x July 13, 2021 15:17
Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

LGTM with nits. Could you rebase this onto the 4.0.x branch, please?

'sphinxcontrib-devhelp>=1.0.2',
'sphinxcontrib-jsmath>=1.0.1',
'sphinxcontrib-htmlhelp>=2.0.0',
'sphinxcontrib-serializinghtml>=1.1.5',
Copy link
Member

Choose a reason for hiding this comment

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

IMO, it's enough to pin only htmlhelp and serializinghtml.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like we should pin them all, so that we don't hit this issue in the future? I imagine there will also be breaking changes over time, and we want users to have the latest version. I don't feel strongly though.

Copy link
Member

Choose a reason for hiding this comment

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

-1: too strict

tk0miya and others added 2 commits July 13, 2021 11:22
@ericholscher
Copy link
Contributor Author

ericholscher commented Jul 13, 2021

I rebased on 4.1.x, since 4.0.x doesn't exist:

-> git pull --rebase origin 4.0.x
fatal: Couldn't find remote ref 4.0.x

@tk0miya
Copy link
Member

tk0miya commented Jul 14, 2021

OMG, this still contains commits on 4.x branch...

@tk0miya tk0miya closed this Jul 14, 2021
@tk0miya
Copy link
Member

tk0miya commented Jul 14, 2021

I'm fixing this in another PR. Thanks,

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
3 participants