-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add release version number to navbar #1083
Conversation
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
Could you update this PR to also update the config.toml file with the current release, as if it were run previously? |
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
.github/scripts/release-docs.sh
Outdated
@@ -53,6 +53,11 @@ VERSION_STRING_REPLACEMENT="version = \"${CHANNEL_VERSION}\"" | |||
awk -v REPLACEMENT="${VERSION_STRING_REPLACEMENT}" '{gsub(/version = \"edge\"/, REPLACEMENT); print}' docs/config.toml > docs/config.toml.tmp | |||
mv docs/config.toml.tmp docs/config.toml | |||
|
|||
# In docs/config.toml, change version to CHANNEL_VERSION instead of latest or older version | |||
DOC_VERSION_STRING_REPLACEMENT="version = \"${CHANNEL_VERSION}\"" | |||
awk -v REPLACEMENT="${DOC_VERSION_STRING_REPLACEMENT}" '{gsub(/^[ \t]*version = "(latest|v.*)"/, REPLACEMENT); print}' docs/config.toml > docs/config.toml.tmp |
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.
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.
Yes, but the indentation is a stylistic choice here it doesn't affect the outcome, if you run the hugo server
command the indentation has no effect on the variable value as it's just a text file. And looking at alternatives I can add an indentation to the awk
command but that will end up indentation the other version
variable above. So, it's just a stylistic choice.
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.
Please see the toml schema for more details: https://github.com/toml-lang/toml?tab=readme-ov-file#example
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.
Hey i fixed it by changing the command!
Here's an explanation of the new one:
In this sed command:
-i.bak edits the file in place and creates a backup with the .bak extension.
-E enables extended regular expressions, which allow for more complex patterns.
The pattern ^( *)[ \t]version = "(latest|v.)" is used to match lines with version = "latest" or version = "v..."
^( )[ \t] captures leading spaces (or tabs) before version = and stores them in \1.
\1 in the replacement part puts the captured indentation back, ensuring the replacement line has the same leading whitespace as the original.
${DOC_VERSION_STRING_REPLACEMENT} is inserted after the captured whitespace.
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.
There's a line after this to delete the tmp file created to pick up the whitespaces
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
Will the .bak file be cleaned up? Or will it be checked into the repo? |
Signed-off-by: jasonviviano <[email protected]>
Signed-off-by: jasonviviano <[email protected]>
Added a line to clean it up! |
Thanks for taking a look at this and attempting a fix! I recommend we work with the engineering team and @sylvainsf on tackling this, as we're adding workarounds on top of workarounds. |
Thank you for helping make the Radius documentation better!
Please follow this checklist before submitting:
In addition, please fill out the following to help reviewers understand this pull request:
Description
Currently we display
latest
as the version label to users when accessing the navigation bar:This PR changes that to an actual version number so users are also more aware of the Radius release and exactly what version of docs they are accessing.
Issue reference
#556
Testing
I ran a shell script locally on a copy of the config.toml file the only lines changed are the chart_version param and version param
For a more specific call out the line:
Looks for a line that contains
version = "latest"
orversion = "v..."
while accepting white spaces or tabs but not params such astag_version
which would be caught without the strict expressionThe shell script modifications i made:
The config.toml file result