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

Build: disable Sphinx manipulation #11441

Merged
merged 6 commits into from
Jul 3, 2024
Merged

Conversation

humitos
Copy link
Member

@humitos humitos commented Jun 27, 2024

Add a feature flag called DISABLE_SPHINX_MANIPULATION that:

  • don't install readthedocs-sphinx-ext Python package
  • don't append anything to the Sphinx's conf.py file
  • enable Read the Docs Addons on these versions automatically

The idea behind this is start defaulting new projects to Read the Docs Addons without breaking old projects.

This is a potential first step in favor of the full deprecation/removal of the Sphinx manipulation (as we already did for MkDocs). Once this happens, building on Read the Docs will produce the exact same result than building locally.

Build page Documentation
Screenshot_2024-06-27_12-01-55 Screenshot_2024-06-27_12-41-39

As you can see in the screenshots, the build process doesn't install the readthedocs-sphinx-ext anymore and doesn't append anything to the Sphinx's conf.py file. Besides, when loading the documentation, the addons are already enabled and showing the new expected behavior.

Related readthedocs/addons#72

Add a feature flag called `DISABLE_SPHINX_MANIPULATION` that:

- don't install `readthedocs-sphinx-ext` Python package
- don't append anything to the Sphinx's `conf.py` file
- enable Read the Docs Addons on these versions automatically

The idea behind this is start defaulting new projects to Read the Docs Addons
without breaking old projects.

This is a potential first step in favor of the full deprecation/removal of the
Sphinx manipulation (as we already did for MkDocs). Once this
happens, **building on Read the Docs will produce the exact same result than
building locally**.

Related readthedocs/addons#72
This is a useful variable that we will require during the deprecation of the
Sphinx context manipulation.

Besides, it seems a useful variable that we should expose to users anyways so
they can use it as relative to calculate other useful paths.
@humitos humitos marked this pull request as ready for review July 1, 2024 14:58
@humitos humitos requested review from a team as code owners July 1, 2024 14:58
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Simple enough.

@@ -654,6 +662,7 @@ def get_rtd_env_vars(self):
# "READTHEDOCS_GIT_HTML_URL": self.data.project.remote_repository.html_url,
"READTHEDOCS_GIT_IDENTIFIER": self.data.version.identifier,
"READTHEDOCS_GIT_COMMIT_HASH": self.data.build["commit"],
"READTHEDOCS_PRODUCTION_DOMAIN": settings.PRODUCTION_DOMAIN,
Copy link
Member

Choose a reason for hiding this comment

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

This kinda feels like an internal thing, especially not documenting it. I assume it's needed for something downstream?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is required by the sphinx-build-compatibility extension to know if we have to perform the APIv2 call to community or business. I may miss the documentation for this, but I will add it in that case. Seems useful to pass it to the build commands.

@humitos humitos enabled auto-merge (squash) July 3, 2024 10:38
@humitos humitos merged commit 2aeea6f into main Jul 3, 2024
5 checks passed
@humitos humitos deleted the humitos/disable-sphinx-manipulation branch July 3, 2024 10:54
humitos added a commit that referenced this pull request Jul 17, 2024
humitos added a commit that referenced this pull request Jul 17, 2024
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.

2 participants