Remove system-wide installation of Sphinx #3185
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Installing Sphinx using the system package manager is considered bad practice. This commit removes that from the dependency installer scripts.
github-actions
bot
added
scripts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Installing Sphinx using the system package manager goes against VTR's common practice. Both VTR and VTR documentation advises users to use a python virtualenv to install python dependencies and have a requirements.txt file to install any python dependencies. The current system-wide dependency installer scripts install Sphinx using the system package managers (APT and DNF) which might be undesirable for users that wish to keep their global python packages clean.
Other than that, it is not the only dependency for building the documentation and users still need to use pip and $VTR_ROOT/docs/requirements.txt to install the other dependencies for building the docs, which include the correct version of Shpinx. This PR removes the redundant and dangerous Sphinx installation from VTR's system-wide package installer scripts.