-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Don't install pygments #7490
Don't install pygments #7490
Conversation
Sphinx declares that dependency in the latest version https://github.com/sphinx-doc/sphinx/blob/bec30596a8f7487cad23df532bf02d1b29024dbc/setup.py#L25 and since 1.0 https://github.com/sphinx-doc/sphinx/blob/690fc257e9a539a4c193ba8efe5587f7c102785c/setup.py#L47
008b396
to
78c9843
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! However, this will affect all the builds so I think we should make this a feature flag so we can rollout this smoothly without breaking everything and being safe.
Note that many projects were recently broken by the absence of this change (#7492) |
It's installed as a dependency of sphinx, not sure if we need a feature flag for this. Also, builds are broken bc of this. |
@ericholscher should have more context for that change |
Not sure -- that was a long time ago. I think it should be fine to unpin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems safe enough 👍
Sphinx declares that dependency in the latest version https://github.com/sphinx-doc/sphinx/blob/bec30596a8f7487cad23df532bf02d1b29024dbc/setup.py#L25
and since 1.0 https://github.com/sphinx-doc/sphinx/blob/690fc257e9a539a4c193ba8efe5587f7c102785c/setup.py#L47
ref #7480
fixes #7492