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 jinja2 pinning on Sphinx 1.8 feature flagged projects #9036

Merged
merged 2 commits into from
Mar 29, 2022

Conversation

ericholscher
Copy link
Member

@ericholscher ericholscher commented Mar 24, 2022

Sphinx 3.1.0 breaks Sphinx 1.8.6 projects,
so we want to avoid another major issue like with docutils breakage in the past.

Sphinx 3.1.0 breaks Sphinx 1.8.6 projects,
so we want to avoid another major issue like with docutils breakage in the past.
@ericholscher ericholscher requested a review from a team as a code owner March 24, 2022 15:40
@ericholscher ericholscher changed the title Don't use master branch explicitly in requirements Fix jinja2 pinning on Sphinx 1.8 feature flagged projects Mar 24, 2022
@ericholscher ericholscher added the PR: hotfix Pull request applied as hotfix to release label Mar 24, 2022
Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Looks good to fix the immediate problem. I'd like to not have these cases hardcoded here, but I know they are needed, unfortunately.

@@ -498,7 +500,7 @@ def _install_mamba(self):

def setup_base(self):
conda_env_path = os.path.join(self.project.doc_path, 'conda')
version_path = os.path.join(conda_env_path, self.version.slug)
os.path.join(conda_env_path, self.version.slug)
Copy link
Member

Choose a reason for hiding this comment

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

This line seems can be completely removed because we are not assigning it to any variable.

@agjohnson
Copy link
Contributor

agjohnson commented Mar 29, 2022

This has been hotfix for bit, so I'm going to merge this as-is. I want to avoid having a hotfix PR half-applied in multiple releases. We can treat follow up clean up and fixing tests as a separate issue, as we won't be hotfixing test fix or cleanup commits

@agjohnson agjohnson enabled auto-merge (squash) March 29, 2022 06:17
@agjohnson agjohnson disabled auto-merge March 29, 2022 06:18
@agjohnson agjohnson merged commit 6bcf611 into main Mar 29, 2022
@agjohnson agjohnson deleted the fix-jinja-sphinx18 branch March 29, 2022 06:18
humitos added a commit that referenced this pull request Nov 23, 2022
Initial file to start documenting hosting integrations.

Related #9036
Related readthedocs/meta#71
Related readthedocs/meta#35
humitos added a commit that referenced this pull request Nov 23, 2022
Initial file to start documenting hosting integrations.

Related #9036
Related readthedocs/meta#71
Related readthedocs/meta#35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: hotfix Pull request applied as hotfix to release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants