-
Notifications
You must be signed in to change notification settings - Fork 22
Fix wording on patch-family upgrade pages (4.2.2.x, 4.3.1.x) #428
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
base: main
Are you sure you want to change the base?
Changes from all commits
a2373cd
112b33c
23e9d85
5de6f49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ export interface UpgradeVersion { | |
| releaseDatePe?: string; | ||
| /** "upgradable-from" value, e.g. "4.2.1.x" or "4.1.0" */ | ||
| upgradableFrom: string; | ||
| /** Optional override for the in-family patch label used in "or any X patch" text and the "from version X" upgrade-script note. When unset, templates fall back to baseVersion (and baseVersion.x for the script note). */ | ||
| patchableFrom?: string; | ||
| /** Anchor of the upgradable-from version on the same platform page */ | ||
| prevVersionAnchor?: string; | ||
| lts: boolean; | ||
|
|
@@ -39,6 +41,17 @@ export function getFamilySlug(family: string): string { | |
| return 'v' + family.replace(/\./g, '-') + '-x'; | ||
| } | ||
|
|
||
| /** | ||
| * All upgrade-eligible versions, newest-first. | ||
| * | ||
| * ORDERING IS LOAD-BEARING — keep entries in descending version order, and within | ||
| * a baseVersion group list the newest patch first. Consumers rely on array position: | ||
| * - UPGRADE_FAMILIES dedups by position to derive family order | ||
| * - UpgradeTable / *UpgradeSteps render rows in array order | ||
| * - LATEST_PATCH_VERSIONS treats the first entry per baseVersion as the latest | ||
| * An out-of-order insert silently mislabels "(latest patch)" and visibly reorders | ||
| * the upgrade table and steps. | ||
| */ | ||
| export const UPGRADE_VERSIONS: UpgradeVersion[] = [ | ||
| { | ||
| version: '4.3.1.2', | ||
|
|
@@ -47,6 +60,7 @@ export const UPGRADE_VERSIONS: UpgradeVersion[] = [ | |
| baseVersion: '4.3.1', | ||
| releaseDate: 'May 28 2026', | ||
| upgradableFrom: '4.2.1.x', | ||
| patchableFrom: '4.3.x', | ||
| prevVersionAnchor: 'v4-3-0-1', | ||
| lts: true, | ||
| patch: true, | ||
|
|
@@ -63,6 +77,7 @@ export const UPGRADE_VERSIONS: UpgradeVersion[] = [ | |
| baseVersion: '4.3.1', | ||
| releaseDate: 'Mar 31 2026', | ||
| upgradableFrom: '4.2.1.x', | ||
| patchableFrom: '4.3.x', | ||
| prevVersionAnchor: 'v4-3-0-1', | ||
| lts: true, | ||
| patch: true, | ||
|
|
@@ -80,6 +95,7 @@ export const UPGRADE_VERSIONS: UpgradeVersion[] = [ | |
| baseVersion: '4.3.1', | ||
| releaseDate: 'Mar 10 2026', | ||
| upgradableFrom: '4.2.1.x', | ||
| patchableFrom: '4.3.x', | ||
| prevVersionAnchor: 'v4-3-0-1', | ||
| lts: true, | ||
| patch: true, | ||
|
|
@@ -708,3 +724,21 @@ export const UPGRADE_VERSIONS: UpgradeVersion[] = [ | |
|
|
||
| /** Unique version families in order, e.g. ["4.3", "4.2", "4.1", ...] */ | ||
| export const UPGRADE_FAMILIES: string[] = [...new Set(UPGRADE_VERSIONS.map((v) => v.family))]; | ||
|
|
||
| /** | ||
| * Version strings that are the newest patch within their baseVersion family. | ||
| * Relies on UPGRADE_VERSIONS being newest-first per baseVersion (see note there) — | ||
| * the first entry seen for each baseVersion is taken as the latest. | ||
| */ | ||
| export const LATEST_PATCH_VERSIONS: Set<string> = (() => { | ||
| const seen = new Set<string>(); | ||
| const latest = new Set<string>(); | ||
| for (const v of UPGRADE_VERSIONS) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This silently depends on
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Concrete suggestion for the ordering dependency flagged earlier — derive "latest" by comparing version components rather than relying on array position, so an out-of-order insert can't silently mislabel the newest patch: function cmpVersion(a: string, b: string): number {
const pa = a.split('.').map(Number);
const pb = b.split('.').map(Number);
for (let i = 0; i < Math.max(pa.length, pb.length); i++) {
const d = (pa[i] ?? 0) - (pb[i] ?? 0);
if (d) return d;
}
return 0;
}
export const LATEST_PATCH_VERSIONS: Set<string> = (() => {
const byBase = new Map<string, string>(); // baseVersion → newest version
for (const v of UPGRADE_VERSIONS) {
if (!v.patch || !v.baseVersion) continue;
const cur = byBase.get(v.baseVersion);
if (!cur || cmpVersion(v.version, cur) > 0) byBase.set(v.baseVersion, v.version);
}
return new Set(byBase.values());
})();If you'd rather keep the positional approach, at least add a one-line comment stating the "newest patch listed first within each baseVersion" invariant, since nothing currently enforces it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. imho, the concern is overblown: the upgrade instructions should be already ordered, since adding out-of-order version on top of the file is not something that could happen (new release = version number increments) or should happen (you logically never "upgrade" to older version, and the document is also always logically read bottom-to-top). i had Claude trace the consumers to confirm - the newest-first order is already relied on by since that is such a corner case and ordering - although already logical - is not enforced, i think adding comment will suffice - will commit it asap
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done: 5de6f49 |
||
| if (!v.patch || !v.baseVersion) continue; | ||
| if (!seen.has(v.baseVersion)) { | ||
| seen.add(v.baseVersion); | ||
| latest.add(v.version); | ||
| } | ||
| } | ||
| return latest; | ||
| })(); | ||
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.
Worth confirming this against the release policy before it ships. Per
src/content/_includes/docs/releases/release-policy.mdx, the scheme is MAJOR.MINOR.MAINTENANCE.PATCH and the "no upgrade script" guarantee is PATCH-level only — i.e. within the same MAINTENANCE number. HerebaseVersionis exactly the MAINTENANCE level, so4.3.0and4.3.1are two distinct maintenance releases.Setting
patchableFrom: '4.3.x'broadens the caution note ("if you are upgrading from version 4.3.x, DO NOT run the upgrade script") to cover users on4.3.0.xmoving to4.3.1.x— which crosses a maintenance boundary. The policy says maintenance bumps "may require upgrade scripts" and should be done in a maintenance window (its example:4.2.0.x → 4.2.1.0). The original4.3.1.xscope (thebaseVersionfallback) matched the policy exactly.Caveat that cuts the other way:
4.3.1is the Angular-20 maintenance release (release notes #14944), i.e. the 4.3 analog of4.2.2.0, which the policy explicitly calls out as "no DB scripts required." So the broadening might be intentional and correct. But then it's inconsistent — this same PR leaves4.2.2.xat4.2.2.xscope (nopatchableFrom). Either both Angular-maintenance lines should be broadened, or neither.Could you confirm whether
upgrade.shhas an incremental4.3.0 → 4.3.1step, or only the step from the4.2.1.xLTS base? If the former, this should stay4.3.1.x. (SamepatchableFrom: '4.3.x'is set on all three 4.3.1.x entries — lines 52, 69, 87.)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.
yep, upgrade script is not needed, and that was actually the main focus of these changes.
current upgrade notes say "do not run starting from 4.3.1.x" - user upgrading from 4.3.0.x may misinterpret that script is required, run
upgrade.sh, see "cannot upgrade this version" and wonder how to proceed.actually - there were real customer portal tickets with questions on such confusions.
a) separate Angular note exists, warning the user about this breaking change - also, 'patchableFrom' is strictly about "should you run
upgrade.shor not"b) 4.2.x upgrades were consciously untouched since current cautions/notes are valid; additionally, these changes to upgrade pages were done with defaults and fallbacks to ensure they work in the same way as before in case 'patchableFrom' wasnt provided