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

Config file: make sphinx or mkdocs configuration required for projects using Sphinx or MkDocs #11852

Merged
merged 7 commits into from
Dec 23, 2024

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Dec 13, 2024

  • This introduces a breaking change for users overriding the new (undocumented) jobs (create_environment, install, build.html and friends). This is, if they have overridden any of those jobs without explicitly declaring a Sphinx or MkDocs key, we will no longer run the sphinx/mkdocs commands, not their setup.
  • This hardcodes the dates from Post: deprecate config files without sphinx or mkdocs configuration website#342 to through an error to users using the configuration file without an explicit sphinx/mkdocs configuration.

This allows for users to keep using the new overrides without worrying about sphinx/mkdocs, while giving enough time to old users to migrate their projects to give an explicit path.

Some notes

  • We are allowing to use sphinx/mkdocs and probably other keys with build.commands, even if those keys don't affect the build in anything. Not really something that "interrupt" users in any way, but it can be missleading.
  • A next step should be to not make python required at all, right now we still create a virtual environment. We probably want to create a virtual env only if python was provided in the list of build.tools.

This comment was marked as off-topic.

@stsewd stsewd marked this pull request as ready for review December 13, 2024 03:24
@stsewd stsewd requested review from a team as code owners December 13, 2024 03:24
@stsewd stsewd requested a review from ericholscher December 13, 2024 03:24
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.

This looks great to me. It seems like we're unblocking ourselves for supporting build.jobs.build for other projects while doing the deprecation here?

@humitos
Copy link
Member

humitos commented Dec 16, 2024

I took a quick look at this code and it seems to be correct. However, I've tried running the Docusaurus example from #11810 and it still breaks with TypeError: sequence item 0: expected str instance, NoneType found when running create_environment. I understand that the job build.jobs.create_envronment should not be run at all in this case.

Adding the following chunk makes that example to work correctly:

diff --git a/readthedocs/doc_builder/director.py b/readthedocs/doc_builder/director.py
index ce93c967c..fab8ecfd6 100644
--- a/readthedocs/doc_builder/director.py
+++ b/readthedocs/doc_builder/director.py
@@ -307,6 +307,12 @@ class BuildDirector:
         if self.data.config.build.jobs.create_environment is not None:
             self.run_build_job("create_environment")
             return
+
+        # If the builder is generic, we have nothing to do here,
+        # as the commnads are provided by the user.
+        if self.data.config.doctype == GENERIC:
+            return
+
         self.language_environment.setup_base()
 
     # Install

I think we need to do something similar for those jobs we don't want to run when it's a GENERIC builder.

We updated these dates there to move faster.
@humitos
Copy link
Member

humitos commented Dec 17, 2024

I implemented my previous comment in #11863.

Continues the work done in #11852 to allow builds without `sphinx` or
`mkdocs` config keys to work.
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 fine. This is ready to merge 👍🏼

pitufo13aguilar

This comment was marked as off-topic.

@stsewd stsewd merged commit 67b01e2 into main Dec 23, 2024
7 of 8 checks passed
@stsewd stsewd deleted the deprecate-conf-file branch December 23, 2024 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants