Skip to content

Remove the workaround for variable substitution in multiversion documentation#3083

Merged
scylla-operator-bot[bot] merged 2 commits intoscylladb:masterfrom
rzetelskik:sphinx-multiversion-0.3.3
Nov 24, 2025
Merged

Remove the workaround for variable substitution in multiversion documentation#3083
scylla-operator-bot[bot] merged 2 commits intoscylladb:masterfrom
rzetelskik:sphinx-multiversion-0.3.3

Conversation

@rzetelskik
Copy link
Copy Markdown
Member

@rzetelskik rzetelskik commented Nov 18, 2025 •
edited
Loading

Description of your changes: sphinx-multiversion-scylla 0.3.4 fixes handling myst_substitutions for multiple branches. This means we can remove the in-tree hack and define substitutions per branch.

Which issue is resolved by this Pull Request:
Resolves #

/kind cleanup
/priority important-longterm
/cc

@scylla-operator-bot
Copy link
Copy Markdown
Contributor

scylla-operator-bot Bot commented Nov 18, 2025

@rzetelskik: GitHub didn't allow me to request PR reviews from the following users: rzetelskik.

Note that only scylladb members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

Description of your changes: WIP

Which issue is resolved by this Pull Request:
Resolves #

/kind cleanup
/priority important-longterm
/cc

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@scylla-operator-bot scylla-operator-bot Bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 18, 2025
@rzetelskik
Copy link
Copy Markdown
Member Author

rzetelskik commented Nov 18, 2025 •
edited
Loading

/hold
I think this is broken. It throws warnings even for 1.17 (https://prow.scylla-operator.scylladb.com/view/gs/scylla-operator-prow/pr-logs/pull/scylladb_scylla-operator/3083/pull-scylla-operator-master-build-docs-multiversion/1990710571602808832#1:build-log.txt%3A273-401) where the substitutions are defined:

scylla-operator/docs/source/conf.py

Lines 55 to 64 in a2fdeb1

myst_substitutions = {
"productName": "Scylla Operator",
"repository": "scylladb/scylla-operator",
"revision": "master",
"imageRepository": "docker.io/scylladb/scylla",
"imageTag": "2025.1.2",
"enterpriseImageRepository": "docker.io/scylladb/scylla-enterprise",
"enterpriseImageTag": "2024.1.12",
"agentVersion": "3.5.0",
}

(The makefile target should also be configured to fail on warnings, but that's unrelated)

@scylla-operator-bot scylla-operator-bot Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2025
@rzetelskik
Copy link
Copy Markdown
Member Author

rzetelskik commented Nov 18, 2025 •
edited
Loading

^
@annastuchlik would you be able to verify it's not a mistake on my side?

@annastuchlik
Copy link
Copy Markdown

annastuchlik commented Nov 18, 2025

@rzetelskik It was tested on our repo.
@dgarcia360 Can you look into it.

@rzetelskik
Copy link
Copy Markdown
Member Author

rzetelskik commented Nov 18, 2025

@rzetelskik It was tested on our repo.
@dgarcia360 Can you look into it.

@dgarcia360 I also tried to test it locally with a single local branch with all variables present in myst_substitutions and had the same issue. It kind of looks like the substitutions aren't propagated? I don't see any obvious misconfigurations on my part.

@dgarcia360 dgarcia360 mentioned this pull request Nov 22, 2025
Merged
@dgarcia360
Copy link
Copy Markdown
Contributor

dgarcia360 commented Nov 24, 2025

@rzetelskik try updating to sphinx-multiversion-scylla version 0.3.4 (just published). This release fixes an issue where the extension could not ensure that MyST variables loaded before the multiversion read the conf variables.

@rzetelskik rzetelskik force-pushed the sphinx-multiversion-0.3.3 branch from 9ba8e40 to 22e6e00 Compare November 24, 2025 09:18
@rzetelskik rzetelskik force-pushed the sphinx-multiversion-0.3.3 branch from 22e6e00 to 996e8e5 Compare November 24, 2025 09:22
@scylla-operator-bot scylla-operator-bot Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 24, 2025
@rzetelskik
Copy link
Copy Markdown
Member Author

rzetelskik commented Nov 24, 2025

Seems to be working fine now. Thanks @dgarcia360!
/hold for updating the substitutions in all branches

This was referenced Nov 24, 2025
Merged
Merged
Merged
@rzetelskik
Copy link
Copy Markdown
Member Author

rzetelskik commented Nov 24, 2025

Requires:

@rzetelskik
Copy link
Copy Markdown
Member Author

rzetelskik commented Nov 24, 2025

/test build-docs-multiversion

@rzetelskik rzetelskik changed the title [WIP] Remove the workaround for variable substitution in multiversion documentation Remove the workaround for variable substitution in multiversion documentation Nov 24, 2025
@scylla-operator-bot scylla-operator-bot Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 24, 2025
@rzetelskik
Copy link
Copy Markdown
Member Author

rzetelskik commented Nov 24, 2025

/hold cancel
/cc czeslavo

@scylla-operator-bot scylla-operator-bot Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 24, 2025
czeslavo
czeslavo approved these changes Nov 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@czeslavo czeslavo left a comment

Choose a reason for hiding this comment

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

/lgtm

@scylla-operator-bot scylla-operator-bot Bot added the lgtm Indicates that a PR is ready to be merged. label Nov 24, 2025
@scylla-operator-bot
Copy link
Copy Markdown
Contributor

scylla-operator-bot Bot commented Nov 24, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: czeslavo, rzetelskik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [czeslavo,rzetelskik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@scylla-operator-bot scylla-operator-bot Bot merged commit 45ba33e into scylladb:master Nov 24, 2025
11 checks passed
@rzetelskik rzetelskik deleted the sphinx-multiversion-0.3.3 branch November 24, 2025 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@czeslavo czeslavo czeslavo approved these changes

Assignees

@czeslavo czeslavo

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rzetelskik @annastuchlik @dgarcia360 @czeslavo