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.

Sign up for GitHub

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

Jump to bottom

GH-38240: [Docs] version_match should match the version from versions.json #38241

Merged
merged 3 commits into from
Oct 17, 2023
Merged

GH-38240: [Docs] version_match should match the version from versions.json #38241

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
11bc973
Change version_match version
AlenkaF Oct 12, 2023
461841c
Use full versions.json url
AlenkaF Oct 12, 2023
26a6561
Revert json_url change
AlenkaF Oct 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions docs/source/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,14 @@
# documentation.
#

switcher_version = version
if ".dev" in version:
switcher_version = "dev/"
else:
# If we are not building dev version of the docs, we are building
# docs for the stable version
switcher_version = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realize a potential problem with this .. We initially build the documentation as the stable docs for the release, but then afterwards we copy those those docs into a subdirectory to keep them as older docs, at which point they are not the "stable" docs anymore ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, I have been thinking about this today and am getting very confused =D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we will need to check for the numbered version of the docs here (13.0.0, 14.0.0, etc) and set the switcher_version to the number extracted from the version string (if not dev) so that once the docs get copied into the subdirectory will be correctly and equal to the version in the versions.json.

Then all the docs versions (old and dev), except stable, should have the correct text in the version switcher button. For the stable version - should we open a PR upstream to change the default text? 😊

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, can we just include both entries in the json file? Both one with "" for the old docs, and one with the actual version for the stable docs, so something like:

    {
        "name": "13.0 (stable)",
        "version": "13.0.0/",
        "url": "https://arrow.apache.org/docs/",
        "preferred": true
    },
    {
        "name": "13.0 (stable)",
        "version": "",
        "url": "https://arrow.apache.org/docs/",
    },

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although that would then give a duplicated entry in the dropdown, so also not ideal I suppose.

Another option would be to patch the doc sources when moving them from / to /xx.x when doing a release. It touches every file, but it's also a quite simple patch. A simple replacement of DOCUMENTATION_OPTIONS.theme_switcher_version_match = ''; with the correct version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am in favour of the second approach (patch the doc sources when moving) 👍
So this PR can be merged as is, the patch can be added as a follow up after the release, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all, I would maybe just merge this PR as is, and then deal with the remaining issue discussed above to when we actually did a release, then it will be easier to experiment with this


html_theme_options = {
"show_toc_level": 2,
"use_edit_page_button": True,
Expand All @@ -312,8 +320,8 @@
],
"show_version_warning_banner": True,
"switcher": {
"json_url": "/docs/_static/versions.json",
"version_match": version,
"json_url": "https://arrow.apache.org/docs/_static/versions.json",
AlenkaF marked this conversation as resolved.
Show resolved Hide resolved
"version_match": switcher_version,
},
}

Expand Down