[Claude code generated PR] Extract isLatestInMajor helper in build-cli#26589
Conversation
…aily-repo-status.md-4130 Add agentic workflow daily-repo-status
…ode-simplifier.md-1519 Add agentic workflow code-simplifier
…uplicate-code-detector.md-4767 Add agentic workflow duplicate-code-detector
…to vermadhr/ghawWorkflowsForSimplification
…ication Vermadhr/ghaw workflows for simplification
…icate deployment logic Extracts the repeated "latest stable version by major" computation from check/latestVersions and vnext/check/latestVersions into a shared pure helper in src/library/latestVersions.ts. Adds unit tests covering all result branches. Closes microsoft#26536 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors build-cli’s “latest stable version by major” decision logic by extracting it from two check commands into a shared pure helper, and adds unit coverage for the helper.
Changes:
- Added
isLatestInMajorhelper (and result type) undersrc/library/latestVersions.ts. - Updated both
check:latestVersionsandvnext:check:latestVersionscommands to delegate to the helper while preserving existing log/ADO variable output. - Added unit tests covering main outcome branches for the helper.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| build-tools/packages/build-cli/src/test/library/latestVersions.test.ts | Adds unit tests for the new helper’s decision outcomes. |
| build-tools/packages/build-cli/src/library/latestVersions.ts | Introduces shared helper and result type for “latest in major” evaluation. |
| build-tools/packages/build-cli/src/commands/vnext/check/latestVersions.ts | Replaces inline logic with a call to isLatestInMajor. |
| build-tools/packages/build-cli/src/commands/check/latestVersions.ts | Replaces inline logic with a call to isLatestInMajor. |
| inputVersion: string, | ||
| ): LatestVersionCheckResult { | ||
| const stableVersions = allVersions | ||
| .filter((v) => !isInternalVersionScheme(v)) |
There was a problem hiding this comment.
isLatestInMajor is documented as checking the latest stable version and says the input list may include prerelease versions, but the implementation only filters out Fluid internal-version-scheme values. Non-internal semver prereleases (e.g. 1.2.3-beta.1) will be treated as stable candidates, which can cause incorrect "latest" results if such tags exist. Either filter out semver.prerelease(v) as well, or update the docstring/parameter description to match the actual behavior.
| .filter((v) => !isInternalVersionScheme(v)) | |
| .filter((v) => !isInternalVersionScheme(v) && semver.prerelease(v) === null) |
| }); | ||
|
|
||
| it("filters out internal/prerelease versions", () => { | ||
| const result = isLatestInMajor(["1.0.0", "2.0.0-internal.1.0.0", "1.2.3"], "1.2.3"); |
There was a problem hiding this comment.
The test "filters out internal/prerelease versions" only includes an internal-version-scheme example and does not include a non-internal semver prerelease (e.g. 1.2.4-beta.1). As written, it won't catch regressions around prerelease filtering; either add a prerelease case or rename the test to reflect what it actually asserts.
| const result = isLatestInMajor(["1.0.0", "2.0.0-internal.1.0.0", "1.2.3"], "1.2.3"); | |
| const result = isLatestInMajor(["1.0.0", "2.0.0-internal.1.0.0", "1.2.3", "1.2.4-beta.1"], "1.2.3"); |
Summary
check/latestVersionsandvnext/check/latestVersionsinto a shared pure helperisLatestInMajorinsrc/library/latestVersions.tsTest plan
pnpm run build:compilepassespnpm run testpasses (403/404; pre-existing flaky git lock failure unrelated to this change)pnpm run lint:fixcleancheck:latestVersionsandvnext:check:latestVersionsdelegate toisLatestInMajorwith identical log outputRelated
Closes #26536
🤖 Generated with Claude Code