-
Notifications
You must be signed in to change notification settings - Fork 3.9k
GH-45290: [Docs][Release] Change show_version_warning_banner substitution #46883
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
kou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, why do we need to update DOCUMENTATION_OPTIONS.show_version_warning_banner in HTMLs? It seems that "preferred": true in our versions.json is enough...
arrow/docs/source/_static/versions.json
Line 11 in 8515a08
| "preferred": true |
Co-authored-by: Sutou Kouhei <[email protected]>
I think, though I am not 100% sure - would need to check previous discussions, the reason for this is that Will check older PRs to see if there is any mention of it. |
|
Hm, I think the reason we need sed for the stable/dev docs is because we simply copy/paste the content where the So I think this, unfortunately, has to stay. I do think we need to update the pin on the theme, though! Will try to work on the issue at the end of this week or in the beginning of next: #39759 |
|
I suggest we include this in the |
kou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
|
But I think joris is right here, we use a current version so we should be able to just switch over as long as we maintain the versions.json file?
Edit: Ah, I guess one more edit because we hardcode it to false. +1 on solving this before 21 |
amoeba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Yeah, it is a bit confusing. I will do the post release task and see if any of the |
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit d024a22. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Rationale for this change
Stable and dev docs are not correctly updated and need manual intervention due to the upstream change in version warning banner.
What changes are included in this PR?
Another sed substitution is added.
Are these changes tested?
I have tested locally with
Are there any user-facing changes?
No.