arrow
arrow copied to clipboard
GH-45290: [Docs][Release] Change show_version_warning_banner substitution
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
> sed -i .bak -e '/^[[:space:]]\{8\}DOCUMENTATION_OPTIONS\.show_version_warning_banner =[[:space:]]*$/{
N
s/^\([[:space:]]\{8\}DOCUMENTATION_OPTIONS\.show_version_warning_banner =[[:space:]]*\)\n[[:space:]]\{12\}false;/\1\n true;/g
}' index.html
Are there any user-facing changes?
No.
- GitHub Issue: #45290
BTW, why do we need to update
DOCUMENTATION_OPTIONS.show_version_warning_bannerin HTMLs? It seems that"preferred": truein ourversions.jsonis enough...
I think, though I am not 100% sure - would need to check previous discussions, the reason for this is that show_version_warning_banner configuration option was not available for older versions of the docs (also mentioned in the pydata-sphinx-theme announcements link you have shared.
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 DOCUMENTATION_OPTIONS.show_version_warning_banner is already set in HTML from the previous release process, see https://github.com/apache/arrow/issues/39737#issuecomment-1905758409.
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: https://github.com/apache/arrow/issues/39759
I suggest we include this in the 21.0.0 release. I’ll take care of the post-release docs task (post-09-docs.sh) to ensure it works as expected — and will fix it if it doesn’t. What do you think @amoeba @kou @assignUser
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?
Ah, yes, show_version_warning_banner will also still have to be updated, but only once for the next release, and then afterwards (if we don't patch that anymore), it should be fine I think.
Edit: Ah, I guess one more edit because we hardcode it to false.
+1 on solving this before 21
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?
Yeah, it is a bit confusing. I will do the post release task and see if any of the sed changes can be removed and we could only depend on the versions.json. If that is the case, the sed commands will not have any effect anyways - AFAIU.
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit d024a221409b88ed4dc4aa5bbb09d2101831df43.
There were no benchmark performance regressions. 🎉
The full Conbench report has more details.