Repository navigation
fix(assemble-docs): sort versions.json numerically - #85
Merged
Merged
Conversation
versions.json was sorted with a reverse string comparison, so v10.0 was placed after v4.0 instead of directly below latest. Replace it with a numeric comparison of the version path components. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
marc0olo
added a commit
to dfinity/icp-js-sdk-docs
that referenced
this pull request
Sep 16, 2026
The assemble-docs action sorted versions.json as strings, so v10.0 was appended below v4.0. Reordered to match the fixed action's output (dfinity/ci-tools#85). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Derive the numeric version regex from the one used for validation so the accepted format has a single source of truth, and drop the reliance on unmatched capture groups when padding omitted version parts. Adds the first tests in the repo, on Node's built-in runner so no new dependencies are needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
marc0olo
added a commit
to dfinity/icp-js-sdk-docs
that referenced
this pull request
Sep 16, 2026
Updates the Auth documentation from dfinity/icp-js-auth@1f15640 (v10.0.0). The automation appended `v10.0` below `v4.0` in `versions.json`, because `dfinity/ci-tools/actions/assemble-docs` sorted the entries as strings. Added a commit on top that puts `v10.0` directly under `latest`. The sort itself is fixed in dfinity/ci-tools#85, so future releases land in the right place. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: pr-automation-bot-public[bot] <189003650+pr-automation-bot-public[bot]@users.noreply.github.com> Co-authored-by: Marco Walz <marco.walz@dfinity.org> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Address the unresolved version-precision and workspace test-script issues.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates assemble-docs to sort documentation versions numerically, keeping latest first and synchronizing tests, CI, documentation, and the bundled action.
Changes:
- Adds numeric version comparison and validation tests.
- Updates CI and test scripts.
- Regenerates the action bundle and documents sorting behavior.
File summaries
| File | Summary |
|---|---|
package.json |
Adds the workspace test script. |
actions/assemble-docs/src/versions.ts |
Implements numeric version parsing and comparison. |
actions/assemble-docs/src/versions.test.ts |
Adds version validation and sorting tests. |
actions/assemble-docs/src/upsert-versions-json.ts |
Applies the numeric comparator. |
actions/assemble-docs/README.md |
Documents version ordering behavior. |
actions/assemble-docs/package.json |
Adds the action test script. |
actions/assemble-docs/dist/index.js |
Updates the generated action bundle. |
.github/workflows/self-test.yaml |
Runs tests in CI. |
.github/CONTRIBUTING.md |
Documents testing instructions. |
Review details
- Files reviewed: 8/9 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
adamspofford
approved these changes
Sep 16, 2026
This was referenced Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
upsert-versions-jsonsorted entries withb.path.localeCompare(a.path), a string comparison. Once a project reached a two-digit major,v10.0sorted belowv4.0instead of directly underlatest— see dfinity/icp-js-sdk-docs#247, where the auth v10.0.0 release landed at the bottom of the version dropdown.Replaced with
compareVersionPaths, which compares the numeric parts of the version path.lateststays first; paths that aren't numeric versions (beta,next, …) go last, alphabetically. Omitted parts count as zero, sov10andv10.0.0are the same version.The numeric pattern is derived from the one
isValidVersionalready used, so the accepted format has a single source of truth.Tests
These are the first tests in this repo. They run on Node's built-in runner, which strips the types itself, so there are no new dependencies and no build step —
pnpm test, wired into CI astest:required.Reverting the comparator to the old string sort fails 7 of them.
Note for the reviewer
The test script passes
--disable-warning=MODULE_TYPELESS_PACKAGE_JSON. Node's advisory suggests adding"type": "module"to the action'spackage.json, butdist/index.jsis a CJS bundle, so that would break the action at runtime.Verification
Ran the built
dist/index.jsagainst auth'sversions.jsonas it was before the v10.0 release. The output is byte-identical to the file now committed to dfinity/icp-js-sdk-docs#247.🤖 Generated with Claude Code