arrow icon indicating copy to clipboard operation
arrow copied to clipboard

GH-45290: [Docs][Release] Change show_version_warning_banner substitution

Open AlenkaF opened this issue 5 months ago • 2 comments
trafficstars

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

AlenkaF avatar Jun 23 '25 12:06 AlenkaF

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...

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.

AlenkaF avatar Jun 24 '25 07:06 AlenkaF

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

AlenkaF avatar Jun 24 '25 09:06 AlenkaF

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

AlenkaF avatar Jul 01 '25 09:07 AlenkaF

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

assignUser avatar Jul 01 '25 13:07 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?

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.

AlenkaF avatar Jul 02 '25 05:07 AlenkaF

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.