-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC [PST] fix lock files and add sphinx-design
#28379
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
Conversation
@lesteve the issue is that with a good frequency theme related PRs are merged into main, for good reason. They improve things anyway, and makes merging this branch to I just merged main into the branch, and had to deal with conflicts, which I just took files from I don't think there's a reasonable way to merge conflicts on lock files. Handling conflicts on lock files is like handling conflicts on a pdf or a .ps file. Sure they're text, technically, but they're far from trivial. So I would rather have dependencies on |
I think the approach for solving conflicts could be something like this (untested): #28084 (comment). Once we find the magical incantation this can be put in a The basic idea would be something like this:
If that is too complicated or painful because we want to update the sphinx-pydata-theme branch very often then I am fine with having the changes in It could also be the case that you realise you need another dependency and then you need to do a PR to Side-comment: I am not a huge fan that the branch name |
@Charlie-XIAO just curious, do you actually care about doc-min-dependencies build? If I were you I would just not bother with it for now and maybe find the easiest way to spend as little time with it. The doc-min-dependencies is low-priority and can be sorted out towards the end. For example maybe something like this would work: diff --git a/.circleci/config.yml b/.circleci/config.yml
index 79d0b1064b..6d91f8556e 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -35,7 +35,7 @@ jobs:
keys:
- doc-min-deps-ccache-{{ .Branch }}
- doc-min-deps-ccache
- - run: ./build_tools/circle/build_doc.sh
+ - run: echo "skip doc-min-dependencies for now"
- save_cache:
key: doc-min-deps-ccache-{{ .Branch }}-{{ .BuildNum }}
paths: |
Sorry if I misunderstood what you meant @lesteve. Even if I do not care about
And you're right: I do not really care about |
Yes exactly deploy only depends on doc, in other words, doc-min-dependencies only exist to make sure examples run fine with the minimum supported versions but doc-min-dependencies is never deployed. By having a quick green CI for doc-min-dependencies like I suggested in #28379 (comment), you would avoid the need to update doc-min-dependencies lock-file |
So, this is assuming that we will add dependencies to If this is the correct interpretation, I'm okay with it. @lesteve after you confirm I can open a PR towards |
I don't think this is needed. What I am suggesting is that in Longer-term, once all the other things have been sorted out, feed-back from maintainers has been gathered and there is some consensus, doing the doc-min-dependencies work in your In particular, this avoids updating lock-files for doc-min-dependencies and having to solve conflicts for something that is super low-priority compared to all the other things in the pydata-sphinx-theme work. |
If you are talking about (1) (2) Updating lock files is essentially running But well I'm not strongly against this, so I have pushed the commit |
I am mostly trying to make your life easier by having to update and fix conflicts in one lock-file instead of two ... Your change looks fine, you disabled the doc-min-dependencies build entirely which works. |
Thanks @lesteve, and I'm sorry for any of my misunderstanding. By the way, may I ask why we ship these generated lock files with the repository? I think I have seen a related issue somewhere but I cannot find it now. |
Please note that this PR targets the
new_web_theme
branch!@adrinjalali you may want to look at the discussion in #28084 (comment). This PR is simply updating the lock files, because the dependencies for
pydata-sphinx-theme
were removed due to preferring the lock files onmain
when resolving conflicts. I also addedsphinx-design
because I think there is no need to leave it for a separate PR.For details how things are failing, see the CircleCI log.