chore(docs): add versioned documentation with auto-detection #752
Conversation
|
Documentation. Coverage Reports: |
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
7a38434 to
c95814a
Compare
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
c95814a to
a5d021e
Compare
📝 WalkthroughWalkthroughAdds an executable Node.js script (.github/scripts/merge-versions.mjs) that merges, deduplicates, and sorts documentation versions into a JSON file. Expands CI workflows: a determine-documentation-versions job computes deployment targets and outputs; publish-documentation job downloads build artifacts, prepares latest/major/preview directories, merges versions.json using the new script, and deploys to S3, CloudFront, and optionally GitHub Pages. release.yaml now exposes whether a new release was created and triggers the documentation workflow when appropriate. Adds a client-side redirect (docs/_src/js/version-redirect.js) and registers it in mkdocs.yml. Sequence Diagram(s)sequenceDiagram
actor ReleaseEvent as Release
participant ReleaseWF as .github/workflows/release.yaml
participant PublishWF as .github/workflows/publish-documentation.yaml
participant DetermineJob as determine-documentation-versions
participant PublishJob as publish-documentation
participant ArtifactStore as Build Artifact
participant MergeScript as .github/scripts/merge-versions.mjs
participant S3 as S3 Bucket
participant CloudFront as CloudFront
participant Pages as GitHub Pages
participant Browser as Client
ReleaseEvent->>ReleaseWF: workflow run
ReleaseWF->>ReleaseWF: capture before/after tags
ReleaseWF->>PublishWF: trigger when new_release == true
PublishWF->>DetermineJob: compute deploy targets & version info
DetermineJob-->>PublishWF: outputs (deploy_latest/major/preview, version)
PublishWF->>ArtifactStore: download documentation artifact
PublishWF->>PublishJob: start publishing
PublishJob->>MergeScript: read existing versions.json and new entries
MergeScript-->>PublishJob: merged versions.json
PublishJob->>S3: sync versioned docs and upload versions.json
PublishJob->>CloudFront: (invalidate/update) deploy
PublishJob->>Pages: publish latest (if configured)
Browser->>Browser: version-redirect.js redirects users to appropriate CloudFront path
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/scripts/merge-versions.mjs:
- Around line 82-104: After JSON.parse of existingFile and newFile, validate
that the parsed values are arrays before proceeding: check that existingVersions
is Array.isArray(existingVersions) and newVersions is
Array.isArray(newVersions); if not, log a clear error including which file
(existingFile or newFile) had unexpected structure and exit with non-zero status
(or throw), so mergeVersions and any iteration over existingVersions/newVersions
won't blow up with a confusing error; apply the same validation after parsing
both files and fail-fast with a descriptive message if the input is not an
array.
In @.github/workflows/publish-documentation.yaml:
- Around line 143-147: The workflow uses actions/download-artifact@v5 which is
inconsistent with .github/workflows/release.yaml's actions/download-artifact@v7;
update the download step in this file (the Download documentation artifact job
that calls actions/download-artifact) to use actions/download-artifact@v7 so
both workflows use the same action version to ensure consistent behavior.
- Around line 44-47: The permissions block currently grants "contents: write"
which is unnecessary for the determine-documentation-versions job; update the
job's permissions so "contents: write" becomes "contents: read" (i.e., change
the permissions: contents entry used by the determine-documentation-versions
job), ensuring the job still checks out the repo and runs shell commands but
with read-only repo access.
- Around line 229-231: The sed replacement in the find command truncates the
canonical URL because it matches href="[^"]*" (including the ending quote) and
replaces it with href="$VERSION_URL (missing the closing quote and the original
page path); update the sed expression in the find -exec line to capture the
original path (e.g. capture the value inside href="...") and use that capture in
the replacement so the result becomes href="$VERSION_URL{captured_path}" with a
proper closing quote; in short, change the match to capture the path and change
the replacement to include the capture and the closing quote so canonical links
become complete URLs.
- Around line 186-212: The inline node -e JSON construction using shell
interpolation (NEW_VERSIONS, deploy_latest/preview/major blocks) is fragile and
can break with special characters; instead call the existing merge-versions.mjs
(or use jq) to safely update and emit a JSON file (e.g., new-versions.json)
rather than embedding JS in shell strings: for each conditional (when
needs.determine-documentation-versions.outputs.deploy_latest/preview/major is
"true") invoke merge-versions.mjs (or jq) passing the current NEW_VERSIONS and
the new entry ({version:'latest'|'preview'|MAJOR_VERSION, title:...,
aliases:[]}) so the tool produces a well-formed JSON and write that output back
to NEW_VERSIONS/new-versions.json for subsequent steps.
In @docs/_src/js/version-redirect.js:
- Around line 34-42: The prefix check is too broad: instead of treating '/v' as
a versioned prefix (which matches '/vendor' etc.), tighten the match by
replacing the '/v' entry with a more specific check; update the logic around
versionedPaths/isVersioned to test for either '/v/' (use
fullPath.startsWith('/v/')) or a version-number pattern (use a regex like
/^\/v\d+(\.\d+)*\// to match '/v1/', '/v1.2/', etc.), referencing the variables
fullPath, versionedPaths and isVersioned and ensure the new check is used where
isVersioned is computed so only true version paths prevent redirect.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/scripts/merge-versions.mjs.github/workflows/publish-documentation.yaml.github/workflows/release.yamldocs/_src/js/version-redirect.jsmkdocs.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
mkdocs.yml (1)
219-226: LGTM!The version redirect script inclusion and mike versioning configuration are correctly set up. The
default: latestaligns with the redirect logic that routes users to/latest/paths..github/scripts/merge-versions.mjs (1)
48-63: LGTM!The merge logic correctly uses a Map for deduplication (newer entries overwrite existing ones), and the sorting handles the
latest > preview > vN (descending)order correctly.docs/_src/js/version-redirect.js (1)
23-27: Verify the CloudFront domain matches the deployment configuration.The
CLOUDFRONT_DOMAINconstant is hardcoded here and also in.github/workflows/publish-documentation.yaml. Ensure both values stay in sync when the domain changes..github/workflows/release.yaml (2)
37-64: LGTM!The release detection logic using before/after tag comparison is robust. It correctly handles the case where no tags exist (empty string fallback) and properly detects when semantic-release creates a new tag.
66-74: LGTM!The workflow integration is correct. Using
skip_build: trueis appropriate since artifacts are already available from thebuild-and-testjob that ran earlier in this workflow..github/workflows/publish-documentation.yaml (3)
1-29: LGTM!The workflow triggers and environment configuration are well-structured. The dual input definitions (
workflow_dispatchandworkflow_call) allow both manual invocation and reuse from the release workflow.
245-251: LGTM!The S3 sync correctly omits
--deleteto preserve existing versioned documentation. The--quiet --no-progressflags keep logs clean while errors will still cause the step to fail.
253-273: LGTM!The GitHub Pages deployment is correctly conditioned on
deploy_latestto maintain SEO presence at the root domain while CloudFront hosts all versioned content.
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
a5d021e to
d74ce55
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In @.github/scripts/merge-versions.mjs:
- Around line 48-63: The mergeVersions function needs input validation: ensure
both existingVersions and newVersions are arrays and filter their entries to
only objects with a string "version" property before adding to versionMap;
reject or throw a clear error if the top-level inputs are not arrays, and when
filtering out bad entries include a warning or error message; finally continue
to dedupe and call sortVersions on the filtered/validated merged array
(referencing mergeVersions, version.version, and sortVersions to locate where to
add these checks).
- Around line 12-43: The sort currently treats unknown strings as numeric
(parseVersionNumber returns 999), causing them to win the numeric-descending
branch; change sortVersions to compute an explicit sort key/category: use
VERSIONS_ORDER for named keys (e.g., "latest","preview") with lowest ranks,
detect /^v\d+$/ to assign a numeric-category and extract the version number for
descending comparison, and assign unknown/non-`vN` entries a highest category so
they sort last; then compare by category first, and for the numeric-category
compare by numeric value descending (use parseVersionNumber only for true `vN`
entries).
In @.github/workflows/publish-documentation.yaml:
- Around line 66-87: The deploy job currently exits early and never writes
outputs, letting downstream publish run with empty outputs; change the determine
deployment step (job id deploy) to always compute and write a SHOULD_DEPLOY
output (true/false) plus the existing
DEPLOY_LATEST/DEPLOY_MAJOR/DEPLOY_PREVIEW/MAJOR_VERSION/VERSION outputs to
GITHUB_OUTPUT before exiting, and set SHOULD_DEPLOY=false when skipping instead
of exiting; then gate the publish-documentation job to run only when
needs.deploy.outputs.SHOULD_DEPLOY == 'true'. Apply the same pattern to the
other similar decision blocks referenced (the blocks around the other
occurrences at the mentioned ranges) so outputs are always set and downstream
jobs are properly gated.
- Around line 260-267: The Deploy to S3 step may leave /versions.json cached by
CloudFront; update the workflow so that after or instead of the aws s3 sync in
the "Deploy to S3" step you explicitly upload versions.json with a short
Cache-Control header (e.g., max-age=60) and then sync/upload the remaining files
with longer TTLs, or use aws s3 sync with --exclude 'versions.json' followed by
aws s3 cp deploy-site/versions.json s3://${{ env.VERSIONED_BUCKET_NAME
}}/versions.json --cache-control "max-age=60,public"; reference the "Deploy to
S3" step and the versions.json file when making this change (you can still keep
the existing echo messages and optionally add CloudFront invalidation if you
need immediate propagation).
- Around line 234-252: The sed replacement in the "Update canonical URLs" step
currently overwrites the entire href value and drops the closing quote; update
the sed invocation that runs over deploy-site/*/ to only replace the original
site URL prefix with "$VERSION_URL" while preserving the remainder of the page
path and the trailing quote (i.e., match the existing href value and substitute
only the prefix, keeping the rest of the path and the closing quote intact),
using BASE_URL/VERSION_URL variables in the expression so each version directory
maps to BASE_URL/<version>/<original-page-path>.
- Around line 158-163: Update the GitHub Actions step named "Download
documentation artifact" to use actions/download-artifact@v7 instead of
actions/download-artifact@v5 so it matches the other workflows; locate the step
with uses: actions/download-artifact@v5 and change the version tag to @v7,
leaving the name and path fields (name: pages, path: new-docs) unchanged.
In @docs/_src/js/version-redirect.js:
- Around line 18-22: Update the localhost bypass check around the hostname
variable in version-redirect.js to include other common local/private addresses:
add 0.0.0.0, ::1 (IPv6 loopback), 10.* and the 172.16.0.0–172.31.255.255 range
in the same if that currently checks 'localhost', '127.0.0.1', and '192.168.*'
so local preview setups are not redirected; ensure the check still
short-circuits (returns false) for these additional ranges wherever that
hostname conditional appears (including lines ~34-52).
- Around line 34-74: The redirect logic for GitHub Pages/custom domains
currently always prepends "/latest" causing already-versioned paths (e.g.,
"/v48/..." or "/preview/...") to become "/latest/v48/..."; replicate the
CloudFront check here by defining the same versioned prefixes (e.g.,
['/latest/', '/preview/', '/v']) and detecting if the resolved path (the
variable computed as path or fullPath after stripping the GH Pages subpath)
starts with any of those prefixes; if it is versioned, do not prepend "/latest"
and construct the newUrl using the original path, otherwise prepend "/latest" as
now; update the logic around pathPrefix/path to perform this check and only
apply the "/latest" prefix when the path is not already versioned.
In @mkdocs.yml:
- Around line 217-226: Remove the unused extra.version block from mkdocs.yml
(the nested keys "extra: version: provider: mike default: latest") since
versioning is handled by version-redirect.js and merge-versions.mjs and the
mkdocs theme does not consume this setting; alternatively, if you intend to keep
it for future use, add a clear comment above the extra.version block explaining
it is intentionally unused and that versioning is provided by
version-redirect.js/merge-versions.mjs and the
mkdocs-code-siemens-code-docs-theme.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/scripts/merge-versions.mjs.github/workflows/publish-documentation.yaml.github/workflows/release.yamldocs/_src/js/version-redirect.jsmkdocs.yml
🧰 Additional context used
🧬 Code graph analysis (1)
.github/scripts/merge-versions.mjs (1)
postversion.js (1)
versions(27-27)
🔇 Additional comments (2)
.github/workflows/release.yaml (2)
37-65: Release detection: confirm semantic-release tags are available locally before comparison.
This approach is reasonable, but it assumesnpx semantic-releasecreates the tag in the checked-out workspace sogit describe --tags --abbrev=0reflects it. If semantic-release only pushes tags remotely in some configurations,AFTER_TAGmay stay unchanged and skip docs publish.
23-35: No action required. All GitHub Actions versions and the Node.js version selector are valid and current as of January 2026.
actions/checkout@v6latest: v6.0.1 (Dec 2, 2025)actions/setup-node@v6latest: v6.1.0 (Dec 3, 2025)actions/download-artifact@v7latest: v7.0.0 (Jan 9, 2026)node-version: lts/krypton(Node.js 24.x, Active LTS through Oct 20, 2026)These are standard, supported versions appropriate for this workflow.
Likely an incorrect or invalid review comment.
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
d74ce55 to
e963467
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yaml (1)
23-35: Fix invalidactions/download-artifactversion; other versions and Node.js LTS codename are valid.
actions/download-artifact@v7does not exist; use v6 instead. The versionsactions/checkout@v6andactions/setup-node@v6are the current stable releases. Thenode-version: lts/kryptonis valid (Node.js 24 LTS, effective October 2025).Required fix
- - uses: actions/download-artifact@v7 + - uses: actions/download-artifact@v6
🤖 Fix all issues with AI agents
In @.github/scripts/merge-versions.mjs:
- Around line 48-63: The mergeVersions function currently assumes each entry has
a string .version and calls versionMap.set(version.version, version); add
lightweight validation that each item in both existingVersions and newVersions
is a non-null object with typeof item.version === 'string' before
indexing/deduping, and throw a clear, fail-fast Error (including which array and
the offending item or its JSON) when validation fails; keep the rest of
mergeVersions (the Map usage and final sortVersions(merged) call) unchanged so
behavior is identical for valid inputs.
- Around line 12-42: The sorting treats unknown/non-vN entries incorrectly
because parseVersionNumber() returns 999 for no match; change sortVersions to
use explicit tiered keys: keep VERSIONS_ORDER for known labels (tier 0/1),
detect vN with parseVersionNumber (returning the numeric part or null/-1) and
assign those a numeric-tier (e.g., tier 2 with numeric value), and assign any
other/unknown strings a final tier (e.g., tier 3). Then sort first by tier, and
for tier 2 compare numeric versions descending (higher vN first); reference
functions/values: VERSIONS_ORDER, parseVersionNumber, and sortVersions to
implement these tiered keys and comparisons.
In @.github/workflows/publish-documentation.yaml:
- Around line 69-145: The current early-exit in the "Determine deployment
targets" step (id: deploy) returns exit 0 without setting outputs, so downstream
job publish-documentation still runs; add an explicit output (e.g.
should_deploy) from the deploy step and set it to "false" on the early-exit path
and "true" when a deployment should proceed, then gate the publish-documentation
job with an if that checks needs.<determine-job-id>.outputs.should_deploy ==
'true' (or steps.deploy.outputs.should_deploy == 'true' if you reference the
step), ensuring you set the output via echo "should_deploy=$VALUE" >>
$GITHUB_OUTPUT in both branches so publish-documentation only runs when
should_deploy is true.
- Around line 167-257: The canonical rewrite currently in the "Update canonical
URLs" step (the sed command inside the for loop) replaces the entire href with
"$VERSION_URL" causing every page to have the same canonical; change the sed
expression to only replace the host/prefix and preserve the original path by
capturing the path portion and reusing it (e.g. use a regex like
href="https://[^/]+(/[^"]*)" and replace with href="$VERSION_URL\1"), updating
the sed call in that step (the block that finds "*.html" and runs sed -i.bak) so
each page's canonical becomes "https://<domain>/<version><original-path>" rather
than a single URL for all pages.
- Around line 190-224: The "Generate versions.json" step currently downloads
versions.json via CloudFront (curl to CLOUDFRONT_DOMAIN), which can return stale
data and cause race conditions; move the existing "Configure AWS credentials"
step to run before the "Generate versions.json" job and change the fetch to read
the object directly from S3 (using the AWS CLI/S3 API) instead of CloudFront so
you always get the latest S3 copy before creating new-versions.json and invoking
node .github/scripts/merge-versions.mjs; ensure the S3 bucket/key used matches
what your deploy uses and that the AWS credentials are available to that step.
In @.github/workflows/release.yaml:
- Around line 37-65: The current release check uses git describe --tags
--abbrev=0 which can pick non-release tags; change the BEFORE_TAG and AFTER_TAG
assignments to only consider semver-ish release tags (e.g., those matching v*).
Replace the git describe invocations in the before_release and check_release
steps with a command that finds the latest tag matching the release pattern (for
example, using git tag --list 'v*' sorted by version and taking the first entry,
then passing that to git describe --tags --abbrev=0), and keep the rest of the
logic that writes BEFORE_TAG/AFTER_TAG and compares with
steps.before_release.outputs.before_tag unchanged.
In @docs/_src/js/version-redirect.js:
- Around line 18-74: Update the redirect logic to (1) expand the local-dev
bypass to include 10/8, 172.16/12 ranges and IPv6 localhost (::1) by augmenting
the hostname checks, and (2) avoid double-prefixing or incorrectly handling
GitHub Pages subpaths by: when computing pathPrefix/path, treat a lone repo
segment (e.g., fullPath like "/repo") as an empty path (set path = "/"); then
before prefixing add a version-detection check using the same regex as
isVersioned (e.g., /^\/(latest|preview|v\d+)(\/|$)/) and if that test passes do
not prepend "/latest" (use the existing path as-is); keep the existing /+/
collapse and window.location.replace behavior and reuse symbols
CLOUDFRONT_DOMAIN, isGitHubPages, isCustomDomain, fullPath, pathPrefix, path,
and isVersioned to locate where to apply these changes.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/scripts/merge-versions.mjs.github/workflows/publish-documentation.yaml.github/workflows/release.yamldocs/_src/js/version-redirect.jsmkdocs.yml
🧰 Additional context used
🧬 Code graph analysis (1)
.github/scripts/merge-versions.mjs (1)
postversion.js (1)
versions(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
mkdocs.yml (1)
217-226: This concern is not applicable. Theprovider: mikeconfiguration is correct and working as designed.The configuration setting refers to the schema and location of
versions.json(which follows the mike format), not the build tool used. The workflow confirms this: it explicitly avoids the mike build tool but properly generatesversions.jsonin the expected format viamerge-versions.mjs, deploying versions at root level (/latest/,/preview/,/v1/, etc.). The theme correctly interprets this configuration and renders the version selector without issues.
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
e963467 to
a8a31aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.github/workflows/publish-documentation.yaml:
- Around line 4-6: Replace the temporary wildcard branch config and testing
notes: change the workflow's branches: ['**'] back to branches: [main], remove
the "TEMPORARY: Run on all branches for testing" line and the "TODO: Restore to
branches: [main] before merging" comment so the workflow only runs on main and
no temporary testing config remains; ensure the YAML remains valid after
editing.
- Around line 124-137: Temporary override that forces DEPLOY_PREVIEW for all
branches (the if block checking [[ -z "$DEPLOY_AS_RELEASE" ]] and setting
DEPLOY_PREVIEW="true") must be removed and replaced with the correct branch
check: only set DEPLOY_PREVIEW when not deploying as a release AND we are on the
main branch (use the existing IS_MAIN/branch check or github.ref comparison),
i.e., change the condition that sets DEPLOY_PREVIEW to validate the current
branch instead of unconditionally treating all branches as main; also remove any
other TEMPORARY always-true testing assignments tied to preview behavior so real
branch gating (main vs others) is enforced.
- Around line 88-100: Uncomment and restore the branch-gate block currently
disabled (the lines using the IS_MAIN and FORCE_DEPLOY check and the echo
statements that set deploy_latest, deploy_major, deploy_preview, major_version,
version, and should_deploy, followed by exit 0) so the workflow enforces "if [[
\"$IS_MAIN\" == \"false\" && \"$FORCE_DEPLOY\" == \"false\" ]]; then" gating
again; ensure the echo outputs to $GITHUB_OUTPUT remain intact and the block
exits 0 when not on main and force_deploy is false to prevent deployments from
non-main branches.
- Around line 233-238: The code unnecessarily strips the 'v' into VERSION_NUM
then re-adds it for the title; remove VERSION_NUM and the sed step and build
NEW_VERSIONS using MAJOR_VERSION directly (use --arg version "$MAJOR_VERSION"
--arg title "$MAJOR_VERSION" when calling jq), and delete the now-unused
VERSION_NUM assignment to simplify the block around MAJOR_VERSION and
NEW_VERSIONS.
In @docs/_src/js/version-redirect.js:
- Around line 31-40: The hardcoded CLOUDFRONT_DOMAIN constant
(d2uqfzn4lxgtwv.cloudfront.net) is duplicated outside this file; add a clear
comment above the CLOUDFRONT_DOMAIN declaration explaining that this value is
mirrored in the deployment workflow and must be updated in both places, and/or
point to the README or a single source-of-truth constant (e.g., "See
infra/workflow/…") so future maintainers know the coupling; update the comment
to reference the symbol CLOUDFRONT_DOMAIN and where the canonical value is
maintained.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/scripts/merge-versions.mjs.github/workflows/publish-documentation.yaml.github/workflows/release.yamldocs/_src/js/version-redirect.jsmkdocs.yml
🧰 Additional context used
🧬 Code graph analysis (1)
.github/scripts/merge-versions.mjs (1)
postversion.js (1)
versions(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: e2e (1, 2)
- GitHub Check: test
- GitHub Check: e2e (2, 2)
- GitHub Check: aot
- GitHub Check: build-and-test / e2e (2, 2)
- GitHub Check: build-and-test / e2e (1, 2)
- GitHub Check: build-and-test / test
- GitHub Check: build-and-test / aot
🔇 Additional comments (23)
mkdocs.yml (1)
219-219: LGTM!The version redirect script is correctly added to the extra_javascript configuration. The path matches the file location in the repository.
.github/scripts/merge-versions.mjs (5)
1-11: LGTM!Clean module setup with appropriate shebang for executable Node.js script and clear documentation header explaining the script's purpose.
20-32: LGTM!The sortKey function correctly categorizes versions and uses negative values for descending numeric sort within the versioned category. The approach is clean and maintainable.
41-50: LGTM!The sort comparison logic correctly handles the two-level sorting (category, then value). Since entries in the same category always have consistent value types, the comparison is safe.
55-80: LGTM!The merge logic correctly uses a Map for deduplication with new versions overriding existing ones. The validation for both arrays and individual entries is thorough.
85-146: LGTM!The main function has solid argument validation, appropriate error handling for missing files (ENOENT), and clear logging. The graceful handling of a missing existing versions file is particularly good for initial deployments.
docs/_src/js/version-redirect.js (4)
19-29: LGTM!The local development detection is comprehensive, covering localhost variants and RFC 1918 private address ranges. The
return falseis appropriate as the IIFE doesn't need a meaningful return value—this just exits early.
42-60: LGTM!The CloudFront redirect logic correctly detects versioned paths and redirects unversioned access to
/latest/. The slash normalization and query/hash preservation are properly handled.
82-86: LGTM!The URL construction is correct, and using
window.location.replaceappropriately avoids polluting browser history with redirect entries.
67-77: The path handling correctly guards against root-level edge cases.The condition
fullPath.split('/').length > 2prevents path extraction for GitHub Pages root deployments. A root path/splits to['', ''](length 1), so the pathPrefix extraction is never applied—the path passes through unchanged as/. This is the intended behavior and works correctly for root-level GitHub Pages.The
/versions.jsonexception correctly prevents redirection when the version selector fetches the versions list.Likely an incorrect or invalid review comment.
.github/workflows/release.yaml (4)
19-21: LGTM!The job outputs correctly expose the release detection results for downstream jobs.
37-41: LGTM!Capturing the tag before semantic-release runs is the correct approach to detect if a new release was created. The
2>/dev/null || echo ""fallback handles the case when no tags exist.
52-64: LGTM!The release detection logic correctly compares before/after tags to determine if semantic-release created a new version. The condition
[[ -n "$AFTER_TAG" && "$AFTER_TAG" != "$BEFORE_TAG" ]]properly handles edge cases like no prior tags.
66-74: Secrets are not required for this workflow call.The
publish-documentationworkflow skips thebuild-and-testjob whenskip_build: trueis passed (which thetrigger-documentationjob does), andbuild-and-testis the only job that requiresSIEMENS_NPM_TOKEN,SIEMENS_NPM_USER, andMAPTILER_KEY. Thepublish-documentationjob itself uses AWS credentials via OIDC role assumption, not secrets.Likely an incorrect or invalid review comment.
.github/workflows/publish-documentation.yaml (9)
7-35: LGTM!The workflow inputs are well-structured with clear descriptions. The distinction between
workflow_dispatch(manual with force_deploy option) andworkflow_call(programmatic without force_deploy) is appropriate.
37-41: LGTM!Centralizing infrastructure constants as environment variables is good practice. Note that
CLOUDFRONT_DOMAINhere matches the hardcoded value indocs/_src/js/version-redirect.js.
45-45: Condition handles both input contexts correctly.The condition
!inputs.skip_build && github.event.inputs.skip_build != 'true'correctly handles bothworkflow_call(typed boolean viainputs) andworkflow_dispatch(string viagithub.event.inputs). This dual check is necessary due to how GitHub Actions handles inputs differently between these triggers.
103-122: LGTM!The version validation and extraction logic is correct:
- Validates semver format with optional suffix
- Correctly extracts major version
- Properly detects pre-releases by checking for
-suffix
165-170: LGTM!The job condition correctly ensures deployment only runs when
determine-documentation-versionssucceeds andshould_deployis true. Usingalways()with explicit result checks is the right pattern for this dependency chain.
191-208: LGTM!The conditional deployment to each version directory (latest, major, preview) is correctly gated by the corresponding output flags. The directory structure will be clean and organized.
245-263: LGTM!The canonical URL update correctly replaces the GitHub Pages URL with the versioned CloudFront URL for each version directory. The backup file cleanup with
|| trueis a good defensive pattern.
272-283: LGTM!The S3 deployment strategy is sound:
- OIDC-based authentication is secure
- Separating
versions.jsonupload with a short cache (60s) ensures version selector updates propagate quickly- Using
--quiet --no-progresskeeps workflow logs clean
284-304: GitHub Pages deployment uses original canonical URLs intentionally.The GitHub Pages deployment correctly uses
new-docs(original docs) rather thandeploy-site/latest(with updated canonical URLs). This preserves the originalelement.siemens.iocanonical URLs for SEO, while theversion-redirect.jsscript handles redirecting users to CloudFront.
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
5 times, most recently
from
f22ac74 to
de6990a
Compare
|
@Killusions Do we have reasons not to use const functions in .github/scripts/merge-versions.mjs ? |
@spliffone No, I can switch it |
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
de6990a to
e24203c
Compare
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
4 times, most recently
from
dd8da7a to
639d9cf
Compare
|
Refactored to be a bit faster, follow some of Rayks recommendations and always re-create the versions.json from the dirs instead of the complex merging. My suggestion would be merging it like this (tested, it works perfectly both on main and for releases, /latest and v48.10 is also already built on the bucket so as soon as this is merged to main it will be deployed fully to pages = element.siemens.io. Then in a second phase we can point or domain to the bucket instead and just remove the final push to pages from the CI script, giving use a seamless transition. |
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
6 times, most recently
from
d61a2d9 to
b2cde14
Compare
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
b2cde14 to
9699fbb
Compare
spike-rabbit
left a comment
spike-rabbit
left a comment
There was a problem hiding this comment.
please check next branch handling
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
2 times, most recently
from
0f4bda9 to
c592c9c
Compare
Removed documentation trigger |
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
c592c9c to
815a5bd
Compare
.github/workflows/release.yaml
Outdated
| MAJOR_VERSION="${{ steps.deploy.outputs.major_version }}" | ||
|
|
||
| echo "Updating /$MAJOR_VERSION/..." | ||
| rm -rf "deploy-site/$MAJOR_VERSION" |
There was a problem hiding this comment.
| rm -rf "deploy-site/$MAJOR_VERSION" |
How could this directory already exist here?
There was a problem hiding this comment.
@spike-rabbit Maybe I understand the code wrong, but a release branch will always update the same major version so I believe we should be defensive and assume the possibility the same major version can already exist.
There was a problem hiding this comment.
yes on the bucket, but for sure not locally
.github/workflows/release.yaml
Outdated
| mkdir -p "deploy-site/$MAJOR_VERSION" | ||
| cp -r new-docs/* "deploy-site/$MAJOR_VERSION/" |
There was a problem hiding this comment.
| mkdir -p "deploy-site/$MAJOR_VERSION" | |
| cp -r new-docs/* "deploy-site/$MAJOR_VERSION/" | |
| mkdir "deploy-site" | |
| mv new-docs "deploy-site/$MAJOR_VERSION" |
.github/workflows/release.yaml
Outdated
| role-session-name: element-release-docs | ||
| aws-region: eu-west-1 | ||
|
|
||
| - run: | |
There was a problem hiding this comment.
I don't get why this is needed?
This just does an ls without using the output
There was a problem hiding this comment.
It's to understand what is currently deployed, but sure, can remove.
.github/workflows/release.yaml
Outdated
| VERSIONS=$(echo "$VERSIONS" | jq '. += [{"version": "https://element.siemens.io", "title": "Preview"}]') | ||
|
|
||
| # Write to deploy-site/versions.json | ||
| mkdir -p deploy-site |
There was a problem hiding this comment.
No need to recreate, this for sure already exists
.github/workflows/release.yaml
Outdated
| find "deploy-site/$MAJOR_VERSION" -name "*.html" -type f -exec sed -i.bak \ | ||
| -e "s|<link rel=\"canonical\" href=\"https://element.siemens.io/|<link rel=\"canonical\" href=\"${VERSION_URL}|g" \ | ||
| {} \; | ||
|
|
||
| # Clean up backup files | ||
| find deploy-site -name "*.bak" -type f -delete 2>/dev/null || true |
There was a problem hiding this comment.
| find "deploy-site/$MAJOR_VERSION" -name "*.html" -type f -exec sed -i.bak \ | |
| -e "s|<link rel=\"canonical\" href=\"https://element.siemens.io/|<link rel=\"canonical\" href=\"${VERSION_URL}|g" \ | |
| {} \; | |
| # Clean up backup files | |
| find deploy-site -name "*.bak" -type f -delete 2>/dev/null || true | |
| find "deploy-site/$MAJOR_VERSION" -name "*.html" -type f -exec sed \ | |
| -e "s|<link rel=\"canonical\" href=\"https://element.siemens.io/|<link rel=\"canonical\" href=\"${VERSION_URL}|g" \ | |
| {} \; |
.github/workflows/release.yaml
Outdated
| - run: | | ||
| SITE_URL="https://element.siemens.io/" | ||
|
|
||
| # Update canonical URLs for version directories |
There was a problem hiding this comment.
Please add comment to explain why this is needed
Killusions
force-pushed
the
chore/add-versioned-documentation-with-auto-detection-in-docs
branch
from
815a5bd to
7d8f630
Compare
|
|
Close #675
Preview at: https://d2uqfzn4lxgtwv.cloudfront.net
Test deployment (release and normal pipeline) in #1279 #1280
Adds version selector to choose between preview (main branch) and major versions (v48, v47, etc.).
Architecture: