feat(changelog): add commit history section for package comparisons and enhance styling#4768
feat(changelog): add commit history section for package comparisons and enhance styling#4768vivekv1504 wants to merge 9 commits intowebex:nextfrom
Conversation
vivekv1504
assigned
bhabalan and
Shreyas281299 and unassigned
bhabalan and
Shreyas281299
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
vivekv1504
force-pushed
the
changelog-version-commits
branch
from
def8fda to
217f0ee
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
vivekv1504
force-pushed
the
changelog-version-commits
branch
from
217f0ee to
4b890f7
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Add commit history section for package-level comparisons (walks all stable versions between A and B) - Add webex package commit history for full version comparisons - Add .stat-item.commits CSS rule for commit count badge - Remove unused clearVersionABtn/clearVersionBBtn DOM references Made-with: Cursor
vivekv1504
force-pushed
the
changelog-version-commits
branch
from
4b890f7 to
8f696d1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f696d1659
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (helperSection) helperSection.classList.add('hide'); | ||
|
|
||
| populateComparisonVersions(); |
There was a problem hiding this comment.
When a user clicks Version Comparison from the default single-view flow, the Base/Target <select> controls stay disabled because switchToComparisonViewMode() no longer re-enables them. In index.html both controls are rendered with disabled by default, so this path leaves users unable to pick versions and blocks all manual comparisons (only URL-driven paths that call switchToComparisonMode() re-enable them).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
done changes updated
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3e2e001f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
docs/changelog/assets/js/app.js
Outdated
| } | ||
| return true; | ||
| } | ||
| if (stableA > stableB) { |
There was a problem hiding this comment.
The if (stableA > stableB) guard is doing a plain string comparison, which rejects valid chronological pairs when the target has a two-digit component (for example base 3.9.0 and target 3.10.0 evaluates as true here). In those cases users get a false validation error and cannot run the comparison at all, so this blocks legitimate package/full comparisons around common minor-version boundaries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
done changes updated with code
const allSorted = sortStableVersions([stableA, stableB]); if (allSorted[0] !== stableA) { alert(Base version (${stableA}) must be older than target version (${stableB}). Please swap.); return false; }
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92b08d8dff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
docs/changelog/assets/js/app.js
Outdated
| if (!res.ok) continue; | ||
| changelog = await res.json(); | ||
| } catch (e) { | ||
| console.warn(`Could not fetch changelog for ${stable}:`, e); | ||
| continue; |
There was a problem hiding this comment.
When collecting package commits across stables, failed intermediate fetches are silently skipped (!res.ok and caught exceptions both continue), so users can get a partial commit history that looks valid but is missing entire stable segments. This is reproducible whenever one stable log is temporarily unavailable or misconfigured, and it leads to misleading comparison output instead of an explicit error that the range could not be fully computed.
Useful? React with 👍 / 👎.
… version comparison
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f592178e84
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
docs/changelog/assets/js/app.js
Outdated
| const numA = parseInt(matchA[2], 10); | ||
| const numB = parseInt(matchB[2], 10); | ||
| return numB - numA; // Sort descending (highest first) | ||
| return numA - numB; // Sort ascending (lowest first) |
There was a problem hiding this comment.
This fallback now sorts prerelease suffixes ascending, so findStableVersion() chooses the oldest prerelease instead of the latest one when an exact stable key is absent. In that case the comparison table can show stale package versions and incorrect changed/unchanged status counts (for example, picking -next.1 instead of -next.N), which is the opposite of the helper’s stated intent to prefer the highest prerelease.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fde4a94a5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const stableA = urlParams.get('compareStableA'); | ||
| const stableB = urlParams.get('compareStableB'); | ||
| const packageName = urlParams.get('comparePackage'); | ||
| const versionA = urlParams.get('compareVersionA'); | ||
| const versionB = urlParams.get('compareVersionB'); |
There was a problem hiding this comment.
handleEnhancedComparisonURL() now recognizes only the new compareStableA/B + comparePackage + compareVersionA/B shape, but populateFormFieldsFromURL() in the same file still exits early when the URL contains the older compare= or versionA/versionB parameters. As a result, existing shared links/bookmarks such as ?compare=3.9.0vs3.10.0 no longer auto-load a comparison at all after this change. Please keep parsing the legacy formats here (or stop reserving them earlier) so previously generated permalinks continue to work.
Useful? React with 👍 / 👎.
| selectedPackage, | ||
| comparisonState.cachedChangelogB, | ||
| 'version-b-prerelease-select', | ||
| comparisonState.currentStableB, | ||
| comparisonState.currentStableA === comparisonState.currentStableB |
There was a problem hiding this comment.
When both sides use the same stable, handlePackageChange() passes true for excludeStable on the target selector, so the target dropdown can never pick the exact stable release. That blocks the normal 3.10.0-next.N → 3.10.0 comparison path from the UI and leaves only the reverse 3.10.0 → 3.10.0-next.N direction available, which is backwards for the Base/Target semantics and produces misleading ranges. The stable option needs to remain selectable on the target side for same-stable prerelease comparisons.
Useful? React with 👍 / 👎.
Shreyas281299
left a comment
Shreyas281299
left a comment
There was a problem hiding this comment.
Code cahnges look good for the commit data but I dont understand why have we refactored the code and removed so much code. We can do the cleanup latter. Lets do any cleanup right now, just add the commit data functionality.
| <option value="">Select a Package</option> | ||
| </select> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Why did we remove this?
| <select id="version-b-select" class="full-width" required disabled> | ||
| <option value="">Select target version</option> | ||
| </select> | ||
| <button type="button" id="clear-version-b-btn" class="btn btn-clear-circle" title="Clear target version" aria-label="Clear target version"><svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round"><path d="M18 6L6 18M6 6l12 12"/></svg></button> |
There was a problem hiding this comment.
Why did we remove this clear buttons?
There was a problem hiding this comment.
If the clear buttons needs i will add
| const clearVersionABtn = document.getElementById('clear-version-a-btn'); | ||
| const clearVersionBBtn = document.getElementById('clear-version-b-btn'); |
There was a problem hiding this comment.
Why did we remove these 2 buttons?
| * @param {Object} specificVersions - Optional map of {packageName: specificVersion} | ||
| * @returns {Object} - Map of {packageName: version} | ||
| */ | ||
| const extractPackagesFromVersion = (changelog, specificVersions = null) => { |
There was a problem hiding this comment.
Why did we remov ethis functionality?
| * @returns {Promise<Object>} Comparison data with versionA, versionB, and comparisonData | ||
| * @throws {Error} If fetch fails or comparison fails | ||
| */ | ||
| const fetchAndCompareVersions = async (versionA, versionB) => { |
There was a problem hiding this comment.
Same here why are we removing this?
| * Handle standard comparison URL parameters on page load. | ||
| * Version A/B are pre-filled but disabled until user selects a package (package is required). | ||
| */ | ||
| const loadStandardComparisonFromURL = async (urlParams) => { |
There was a problem hiding this comment.
Why are ew remove this?
why am remove that full version comparison related code becuse that is not recommended this time |
| const isExactStable = (version) => /^\d+\.\d+\.\d+$/.test(version);// | ||
|
|
||
| // Extract numeric suffix: "3.5.0-next.5" → 5, "3.5.0-multipleLLM.3" → 3 | ||
| const getPreReleaseNum = (version) => { |
There was a problem hiding this comment.
[Low] getPreReleaseNum regex matches stable versions
/\.(\d+)$/ would match 3.6.0 and return 0. Currently safe because callers filter with isPreRelease first, but could cause subtle bugs if called in a different context.
Suggested fix: Consider making the regex more specific: /\-[a-zA-Z]+\.(\d+)$/.
There was a problem hiding this comment.
/-[a-zA-Z]+\.(\d+)$/ update thye regex becuase it taking wrons in sometime ,how they are if pre-release version as stable version selects (3.6.0--> it takes 0)as for previous regex ,actually that is wrong .
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7073f598af
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
docs/changelog/assets/js/app.js
Outdated
| versionsToUse = (isExactStable(versionA) && isExactStable(versionB)) | ||
| ? [] | ||
| : all.filter(v => isPreRelease(v, stableVersion)); |
There was a problem hiding this comment.
When both selected versions are exact stables, this branch forces versionsToUse to an empty list for every middle stable. In a compare like 3.6.0 -> 3.10.0, that drops commits from intermediate releases (e.g., 3.7.x, 3.8.x, 3.9.x) and leaves only endpoint data, so the commit-history section underreports changes across the requested range.
Useful? React with 👍 / 👎.
docs/changelog/assets/js/app.js
Outdated
| if (versionA === stableVersion) { | ||
| // both versionA and versionB are exact stables → skip base entirely | ||
| // versionA is stable but versionB is a pre-release → pick stable commits | ||
| versionsToUse = isExactStable(versionB) ? [] : [stableVersion]; |
There was a problem hiding this comment.
For start ranges, if the base is an exact stable and the target is a prerelease, this path includes the base stable entry itself. That makes comparisons like 3.6.0 -> 3.10.0-next.7 include baseline commits that existed at the starting version, so the “between versions” history is overstated with already-released changes.
Useful? React with 👍 / 👎.
docs/changelog/assets/js/app.js
Outdated
| const num = getPreReleaseNum(v); | ||
| // Same tag (e.g. "next"): include if num >= numA | ||
| // Different tag (e.g. "multipleLLM"): include all — alternate pre-release streams also ship in the final stable | ||
| console.log('tag', tag, 'tagA', tagA); |
There was a problem hiding this comment.
Looks like some debug logs got left in here. These fire on every iteration of the filter so they'll spam the console pretty heavily during any comparison. There are a few more at L1051-1052 and L1223 as well — would clean all of these up before merging.
There was a problem hiding this comment.
removed console logs
| const all = new Map(); | ||
| const traversed = []; | ||
|
|
||
| for (let i = 0; i < stables.length; i++) { |
There was a problem hiding this comment.
These intermediate changelogs are fetched one at a time in the loop — if someone compares 3.6.0 to 3.15.0 that's roughly 10 sequential network requests each waiting on the previous one. Could we batch these with Promise.all instead? Something like:
const intermediateStables = stables.filter(s => s !== stableA && s !== stableB);
const fetched = await Promise.all(
intermediateStables.map(async (stable) => {
try {
const res = await fetch(versionPaths[stable]);
return [stable, res.ok ? await res.json() : null];
} catch { return [stable, null]; }
})
);
const changelogMap = Object.fromEntries(fetched);Then just look up changelogMap[stable] in the loop instead of awaiting inside it.
There was a problem hiding this comment.
i am using promise.all fetching at a time files becuase previous it takes 3 to 4 sec take for fetching all files one by one , now its takes 300ms for fetching files
| } else { | ||
| // Full version comparison | ||
| performVersionComparison(stableA, stableB); | ||
| } |
There was a problem hiding this comment.
The old code had an else branch here that called performVersionComparison(stableA, stableB) — that was the path for comparing all packages between two versions without picking a specific one. With that removed, and updateCompareButtonState (L1422) now disabling the button unless a package is selected, the full-version comparison flow is gone entirely.
Was this intentional? If so might be worth calling it out in the PR description since it's a user-facing behavior change.
There was a problem hiding this comment.
"Yes, this is intentional. The full-version comparison (comparing all packages between two stables without selecting a specific package) has been removed. It was rarely useful and could be misleading — the package-level comparison with commit history is the primary workflow now. I 'll call this out in the PR description."
docs/changelog/assets/js/app.js
Outdated
| let changelogA, changelogB; | ||
| if (stableA === stableB) { | ||
| // Same stable — fetch once and reuse for both sides | ||
| changelogA = await fetch(versionPaths[stableA]).then(res => res.json()); |
There was a problem hiding this comment.
This doesn't check res.ok before calling res.json(). If the server returns a 404 or 500 you'll get a confusing SyntaxError from trying to parse HTML as JSON instead of a useful error message. Same thing on L1459-1460.
The fetch in collectCommitsAcrossStables (L1125) already does this check — same pattern would work here:
const res = await fetch(versionPaths[stableA]);
if (!res.ok) throw new Error(`Failed to load changelog for ${stableA} (${res.status})`);
changelogA = await res.json();There was a problem hiding this comment.
add the checks for data fetching are not (Add a res.ok check before parsing JSON. ),Now if the server returns a 404 or 500, the user sees "Error loading version data. Failed to fetch 3.8.0 (404)" instead of a cryptic SyntaxError.
docs/changelog/assets/js/app.js
Outdated
| // Is this version a pre-release of the given stable? | ||
| // e.g. isPreRelease("3.5.0-next.1", "3.5.0") → true | ||
| // isPreRelease("3.5.0", "3.5.0") → false | ||
| const isPreRelease = (version, stableVersion) => { |
There was a problem hiding this comment.
This creates a new RegExp on every call, and it gets called quite a bit inside the filter callbacks. Since we're really just checking if the version starts with the stable prefix + '-', startsWith would do the same thing without the regex overhead:
const isPreRelease = (version, stableVersion) =>
version.startsWith(stableVersion + '-');There was a problem hiding this comment.
clear the regex because it creates new regExp on every call ,added this code instead of regex
version.startsWith(stableVersion + '-');
| // Get all stable versions (from versionPaths) that sit between stableA and stableB (inclusive) | ||
| const getStableVersionsBetween = (stableA, stableB) => { | ||
| const all = sortStableVersions(Object.keys(versionPaths)); | ||
| const iA = all.indexOf(stableA), iB = all.indexOf(stableB); |
There was a problem hiding this comment.
This handles either argument order with Math.min/max, which is nice, but it can cause issues downstream. collectCommitsAcrossStables assigns 'start'/'end' positions by checking stable === stableA vs stable === stableB. If stableA is actually the later version, those positions get swapped and the commit collection logic inverts.
validateComparisonInputs catches this for form submissions, but loadEnhancedComparisonFromURL bypasses validation entirely — so a hand-crafted URL with the versions flipped would hit this. Worth either validating in loadEnhancedComparisonFromURL or normalizing the order at the top of collectCommitsAcrossStables.
There was a problem hiding this comment.
"The UI already validates version order before comparison. loadEnhancedComparisonFromURL only runs from permalinks generated by our own code, which always produces correctly ordered parameters. I'll add a comment noting this assumption."
docs/changelog/index.html
Outdated
| <div class="comparison-helper hide" id="comparison-helper"> | ||
| <p class="comparison-helper-tip"> | ||
| <strong>💡 Tip:</strong> After comparing, you can share the URL from your browser's address bar, or use the "Copy Permalink & share link" button above. | ||
| <strong>💡 Tip:</strong> After comparing, you can share the URL from your browser's address bar, or use the "Copy Permalink & share link" button above. |
There was a problem hiding this comment.
The button on L134 says "Copy Permalink" but this tip text still says "Copy Permalink & share link" — should probably match.
There was a problem hiding this comment.
updated that tip message matched with copy permalink button name
docs/changelog/assets/js/app.js
Outdated
| const tag = getPreReleaseTag(v, stableVersion); | ||
| const num = getPreReleaseNum(v); | ||
| // Same tag: include if num <= numB | ||
| // Different tag: include all-— alternate pre-release streams also ship in the final stable |
There was a problem hiding this comment.
nit: extra dash before the em dash — all-— should be all —
There was a problem hiding this comment.
Fixed the typo — removed the extra dash.
docs/changelog/assets/js/app.js
Outdated
| // Is this an exact stable version (no pre-release suffix)? | ||
| // e.g. isExactStable("3.6.0") → true | ||
| // isExactStable("3.6.0-next.1") → false | ||
| const isExactStable = (version) => /^\d+\.\d+\.\d+$/.test(version);// |
There was a problem hiding this comment.
nit: stray // at the end of this line
There was a problem hiding this comment.
Removed the stray comment marker.
| {{#each commitsBetween}} | ||
| <tr> | ||
| <td><code><a href="{{this.url}}" target="_blank">{{this.shortHash}}</a></code></td> | ||
| <td>{{{github_linking this.message 'message'}}}</td> |
There was a problem hiding this comment.
This uses triple-brace Handlebars (unescaped output). I see the existing template on L248 does the same thing so it's not new, but worth double-checking that github_linking sanitizes its input — a commit message with something like <img onerror=alert(1)> would get rendered as-is otherwise.
There was a problem hiding this comment.
Good catch — added HTML escaping before link injection in github_linking for both the hash and message cases.
When I do this comparision the commits or the packages changed both are incorrect. Please fix this, we dont show any commits for 3.5.0-next.X, 3.6.0-next.X, 3.7.0-next.X, |
…mmit collection, and nits
now ,we can compare with both stable version instead of pre-release versions ,and gets in between version with commits collected
|
4 participants
COMPLETES #< SPARK-772395 >
Vidcast Link:-https://app.vidcast.io/share/40aa3aaa-51cb-470a-850c-10015be09353
This pull request addresses
This pull request addresses
The Webex JS SDK Changelog portal had no way to view the commit history of a specific package between two SDK stable releases. When a user wanted to know what changed in, say, @webex/calling between 3.6.0-next.5 and 3.10.0-next.7, they had to manually open each version log file and scan through commits — across multiple intermediate stable versions (3.7.0, 3.8.0, 3.8.1, 3.9.0) — with no tooling to aggregate them.
by making the following changes
Implemented a cross-stable commit history collection engine in docs/changelog/assets/js/app.js that:
Identifies all stable versions that sit between a user-selected base version (stableA) and target version (stableB) using semver-aware sorting
Fetches each intermediate stable's log file in sequence, reusing already-loaded changelogs for stableA and stableB to avoid redundant network requests
Applies position-based rules for which pre-release entries to include per stable:
start — from the user's base pre-release (e.g. 3.6.0-next.5) through all remaining pre-releases of that stable
middle — skips the exact stable entry (e.g. 3.7.0); collects ALL pre-releases of that intermediate stable
end — from next.1 up to the user's target pre-release (e.g. 3.10.0-next.7) inclusive
only — when both versions fall within the same stable file, collects the range within that single file
Renders results in a table with short-linked commit hash (linked to GitHub), commit message (with PR number auto-linked), and the pre-release version the commit belongs to
Displays a summary of how many commits were collected and which stable log files were traversed
Breaking behavior change: The full-version comparison mode (comparing all packages between two stable versions without selecting a specific package) has been removed
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.