feat(changelog): Enable viewing commit history between two widgets stable releases#661
feat(changelog): Enable viewing commit history between two widgets stable releases#661vivekv1504 wants to merge 18 commits intowebex:nextfrom
Conversation
… app.js and comparison-view.js
…comparison logic in app.js and comparison-view.js
…agement in app.js
… in app.js and comparison-view.j
…ll version comparison related code
…, and clean up code in app.js and comparison-view.js
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4d5b3ca08
ℹ️ 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".
| try { | ||
| const commits = await collectCommitsAcrossStables( | ||
| enhancedParams.packageName, | ||
| enhancedParams.stableA, | ||
| enhancedParams.stableB, |
There was a problem hiding this comment.
This edit removes the old standard-comparison loader block (handleComparisonURLParams / loadStandardComparisonFromURL), but initializeComparisonMode still calls both functions later in app.js. On a normal page load where no enhanced params are present, that path throws a ReferenceError and halts comparison-mode initialization, so legacy ?compare= / ?versionA=&versionB= links stop working entirely.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
upadate with code changes.
loadStandardComparisonFromURL actaually this is for full version comparison url params ,forgot to remove that code .
docs/changelog/index.html
Outdated
| <<<<<<< HEAD | ||
| <label for="comparison-package-select">Package for comparison:</label> | ||
| <select id="comparison-package-select" class="full-width"> | ||
| <option value="">Select a package for comparison</option> | ||
| ======= |
There was a problem hiding this comment.
The HTML still contains Git conflict markers (<<<<<<<, =======, >>>>>>>) in the package selector block, which leaves conflicting duplicate markup in the live page. This creates malformed/ambiguous DOM around the comparison package select and can break or confuse selector behavior in the comparison UI.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
removed merge conflicts markers
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
widgets/docs/changelog/assets/js/app.js
Line 1318 in 6c6bcbe
Restore comparison view handler before wiring click listener
setupComparisonEventListeners still binds the comparison toggle button to switchToComparisonViewMode, but that function was removed in this commit. When initialization reaches this line, JavaScript throws a ReferenceError, which aborts the rest of listener registration (version/package/pre-release selectors, submit, clear, and copy-link handlers), so comparison mode becomes non-functional on load.
ℹ️ 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 numA = parseInt(versionASpecific.match(/\.(\d+)$/)?.[1] || '0', 10); | ||
| const numB = parseInt(versionBSpecific.match(/\.(\d+)$/)?.[1] || '0', 10); |
There was a problem hiding this comment.
The same-stable validation compares trailing numeric segments for both selected versions, but exact stable versions (for example 1.28.2) also match this regex and produce 2 from the patch part. That makes valid flows like base 1.28.2-ccwidgets.7 to target (Stable) 1.28.2 fail with “base pre-release must be older,” even though stable should be treated as newer than any prerelease in that train.
Useful? React with 👍 / 👎.
docs/changelog/index.html
Outdated
| <div class="form-row" id="comparison-package-row" style="display: none;"> | ||
| <div class="form-group"> | ||
| <label for="comparison-package-select">Package (optional for full comparison):</label> | ||
| <<<<<<< HEAD |
There was a problem hiding this comment.
[Critical — Merge Conflict] This line introduces an unresolved merge conflict marker (<<<<<<< HEAD) into the HTML. No matching ======= or >>>>>>> markers appear in the diff, meaning the conflict was partially resolved but this opening marker was left behind. It will render as raw text in the browser, breaking the UI layout. Please resolve this conflict — choose between "Package for comparison:" and "Package (optional for full comparison):" — and remove the marker.
There was a problem hiding this comment.
clear the unresolved conflicts marker in html file
| * Walk every stable between stableA..stableB, | ||
| * fetch each log file, and collect deduplicated commits. | ||
| */ | ||
| const collectCommitsAcrossStables = async (packageName, stableA, stableB, versionA, versionB) => { |
There was a problem hiding this comment.
[Medium — Missing Cache Reuse] This function re-fetches every stable version's changelog via fetch(path), including stableA and stableB, even though comparisonState.cachedChangelogA/B already holds them from handleStableVersionChange. The PR description states "reusing already-loaded changelogs for stableA and stableB" but the implementation doesn't do this.
Suggestion: Pass cachedChangelogA and cachedChangelogB as parameters (or access from comparisonState), and skip fetching when stable === stableA or stable === stableB.
There was a problem hiding this comment.
I agree with these it is refetching even though holds the cachedata from handleStableVersionChange ,and changes to code
if (stable === stableA && comparisonState.cachedChangelogA ) { changelog = comparisonState.cachedChangelogA; } else if (stable === stableB && comparisonState.cachedChangelogB) { changelog = comparisonState.cachedChangelogB; } else {
docs/changelog/assets/js/app.js
Outdated
|
|
||
| let changelog; | ||
| try { | ||
| const res = await fetch(path); |
There was a problem hiding this comment.
[Medium — Silent Error Swallowing] The fetch call doesn't check res.ok before calling res.json(). The old code that was removed had if (!res.ok) throw new Error(...). The bare catch { continue; } prevents crashes but makes failures completely silent — a failed fetch for an intermediate version silently skips commits with no indication to the user or developer.
Suggestion: Add if (!res.ok) check and at minimum console.warn when a stable version's changelog fails to load, so failures are diagnosable.
There was a problem hiding this comment.
add condition for checking the fetch changelog data
if (!res.ok) { throw new Error(Failed to fetch changelog for stable ${stable}); }
docs/changelog/assets/js/app.js
Outdated
|
|
||
| // Skip single-view URL handling if comparison parameters are present | ||
| if ( | ||
| queryParams.has('compare') || |
There was a problem hiding this comment.
[Medium — ?compare= URL Regression] This early return guard still checks queryParams.has('compare'), but handleComparisonURLParams (which processed ?compare= URLs) and loadStandardComparisonFromURL were both deleted. Visiting ?compare=1.27.0vs1.28.0 will now: (1) skip single-view form population because of this early return, (2) NOT load comparison mode because the handler is gone. Result: a blank page.
Suggestion: Either remove queryParams.has('compare') from this guard since it's no longer handled, or add a minimal fallback that redirects or shows a message that the old URL format is no longer supported.
There was a problem hiding this comment.
removing the compare queryParam that is not useful
| if (!isPreRelease(v, stableVersion)) return false; | ||
| const tag = getPreReleaseTag(v, stableVersion); | ||
| const num = getPreReleaseNum(v); | ||
| const afterStart = tag === tagA ? num >= numA : false; |
There was a problem hiding this comment.
[Medium — Cross-Tag Pre-release Returns Zero Commits] When position === 'only' (stableA === stableB) and the two pre-release versions have different tags (e.g., 1.28.0-next.5 vs 1.28.0-ccwidgets.3), this filter requires BOTH afterStart and beforeEnd to be true. Since tag === tagA and tag === tagB can't both be true when tags differ, no commits are ever returned — silent data loss. Additionally, validateComparisonInputs only compares numeric suffixes without checking tag equality, so this scenario passes validation.
Suggestion: Either handle cross-tag (use : true instead of : false to include other tags' pre-releases), or add validation in validateComparisonInputs to require matching pre-release tags.
There was a problem hiding this comment.
replace the true instead of false
|
|
||
| // Update DOM | ||
| comparisonResults.innerHTML = html; | ||
| comparisonResults.innerHTML += html; |
There was a problem hiding this comment.
[Low — Fragile Render Order] Changed from innerHTML = html to innerHTML += html. This works correctly because both call sites call renderCommitHistory first (which resets the container), but this implicit contract is fragile and undocumented. Consider adding a comment explaining the required render order, or building both HTML sections and setting innerHTML once.
There was a problem hiding this comment.
Adding comment for detail information
docs/changelog/assets/js/app.js
Outdated
|
|
||
|
|
||
| Handlebars.registerHelper('math', function (index, offset) { | ||
| return index + offset;// |
There was a problem hiding this comment.
[Low — Trailing Empty Comment] return index + offset;// has a trailing empty comment that looks like a development leftover. Also, the two extra blank lines above this helper definition are unnecessary. Minor cleanup.
There was a problem hiding this comment.
clear the extra empty lines and romoved the empty trailling comments
…pp.js and comparison-view.js
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc692efdc2
ℹ️ 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".
| isExactStable(versionA) && isExactStable(versionB) | ||
| ? [] |
There was a problem hiding this comment.
When both selected boundary versions are exact stables (for example comparing (Stable) 1.28.0 to (Stable) 1.28.2), this branch returns an empty list for every middle stable. That drops all intermediate prerelease commits from the commit-history output, so the new “between stable releases” view can return materially incomplete results for a common comparison flow.
Useful? React with 👍 / 👎.
docs/changelog/assets/js/app.js
Outdated
| } catch { | ||
| continue; |
There was a problem hiding this comment.
If any stable changelog fetch fails, the empty catch silently skips that stable and continues rendering results. This produces partial commit histories without any warning to the user, which can misstate what changed when a log file is missing or temporarily unavailable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good Point, take a look at this. refer to the other comment at this part.
There was a problem hiding this comment.
In catch block is silently skips the error instead of this add the warn console log .
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8151ddcab
ℹ️ 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 same stable and both pre-releases selected, base must be older | ||
| if (stableA === stableB && versionASpecific && versionBSpecific) { |
There was a problem hiding this comment.
This same-stable ordering guard only runs when both pre-release selectors are filled, so a comparison like base=1.28.2 (left selector blank, which defaults to stable) and target=1.28.2-next.5 bypasses validation even though the base is newer than the target. Because handleComparisonSubmit falls back blank selectors to the stable version, this allows inverted ranges to execute and return misleading commit history instead of rejecting the input.
Useful? React with 👍 / 👎.
| const tagB = getPreReleaseTag(versionB, stableVersion); | ||
| const numB = getPreReleaseNum(versionB); | ||
| versionsToUse = all.filter((v) => { | ||
| if (!isPreRelease(v, stableVersion)) return false; |
There was a problem hiding this comment.
In the 'only' branch (same stable file), this filter unconditionally excludes non-prerelease entries. When users compare from a prerelease to the stable endpoint (e.g. base=1.28.2-next.5, target left blank so it defaults to 1.28.2), the stable release entry is never included, so the reported commit history is incomplete and misses the final stable commit.
Useful? React with 👍 / 👎.
|
the PR mentions "Displays a summary of how many commits were collected and which stable log files were traversed". |
docs/changelog/assets/js/app.js
Outdated
|
|
||
| let comparisonMode = false; | ||
| /* ============================================ | ||
| UI HELPER FUNCTIONS |
There was a problem hiding this comment.
can we remove this comment? as the ui helper method are no longer there in the code
There was a problem hiding this comment.
removed that UI helpers commented lines
docs/changelog/assets/js/app.js
Outdated
| } catch { | ||
| continue; |
There was a problem hiding this comment.
Good Point, take a look at this. refer to the other comment at this part.
| } | ||
| } else if (position === 'middle') { | ||
| versionsToUse = | ||
| isExactStable(versionA) && isExactStable(versionB) |
There was a problem hiding this comment.
for scenaro like like v1.28.1 to v1.28.2 , it gets [] ? is this actually how it should work?
There was a problem hiding this comment.
presently there is no stable versions in log files for the future ,new versions are coming with stable versions included it is used
if (versionA === stableVersion) { versionsToUse = all.filter((v)=>isPreRelease(v,stableVersion)); }
updated code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8151ddcab
ℹ️ 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".
| : all.filter((v) => { | ||
| if (!isPreRelease(v, stableVersion)) return false; | ||
| if (!targetTag) return true; | ||
| return getPreReleaseTag(v, stableVersion) === targetTag; | ||
| }); |
There was a problem hiding this comment.
The middle-range branch contradicts the stated rule in this file ('middle' → ALL pre-releases) by filtering intermediate stables to a single targetTag. When a package has multiple prerelease tracks in the intermediate stable (for example next and beta), commits from the non-selected tag are dropped, so the generated cross-stable history is incomplete.
Useful? React with 👍 / 👎.
| const afterStart = tag === tagA ? num >= numA : true; | ||
| const beforeEnd = tag === tagB ? num <= numB : true; | ||
| return afterStart && beforeEnd; |
There was a problem hiding this comment.
In the 'only' branch, afterStart and beforeEnd default to true for versions whose tag is neither tagA nor tagB, so unrelated prerelease tracks are included in the result set. This inflates commit history for same-stable comparisons by pulling commits outside the requested version interval.
Useful? React with 👍 / 👎.
| const numA = parseInt(versionASpecific.match(/\.(\d+)$/)?.[1] || '0', 10); | ||
| const numB = parseInt(versionBSpecific.match(/\.(\d+)$/)?.[1] || '0', 10); | ||
| if (numA > numB) { | ||
| alert('Base pre-release version must be older than target pre-release version'); |
There was a problem hiding this comment.
The same-stable validation compares only the numeric suffix, so a base exact stable version (parsed as 0) and a target prerelease (e.g. .5) passes validation even though stable is chronologically newer. That allows reverse ranges to execute and produces misleading commit-history output instead of blocking invalid input.
Useful? React with 👍 / 👎.
I seen the issue is previous code is there stable group but intensionally i am removed that code then i forgot to update the PR description |
…improving error handling in collectCommitsAcrossStables function
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f90949a7f
ℹ️ 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 (versionA === stableVersion) { | ||
| versionsToUse = all.filter((v)=>isPreRelease(v,stableVersion)); |
There was a problem hiding this comment.
In collectCommitsFromStable, the 'start' branch treats an exact-stable base (versionA === stableVersion) as “include all prereleases,” so ranges like base 1.28.0 → target 1.28.2-next.7 pull in 1.28.0-* commits that happened before the chosen base boundary. This produces inflated and misleading commit history for common stable-to-later comparisons.
Useful? React with 👍 / 👎.
| if (versionB === stableVersion) { | ||
| versionsToUse = [stableVersion]; |
There was a problem hiding this comment.
The 'end' branch currently returns only [stableVersion] when versionB is an exact stable. For comparisons ending at a stable release (for example 1.28.0-next.5 → 1.28.2), this drops all 1.28.2-* prerelease commits that occur between the selected bounds, so the generated commit history is materially incomplete.
Useful? React with 👍 / 👎.
4 participants
COMPLETES #< SPARK-772395 >
vidcast :-https://app.vidcast.io/share/51651bfb-b37f-4863-acbc-931cdeb46e93
This pull request addresses
This pull request addresses:
The Webex Widgets Changelog portal had no way to view the commit history of a specific package between two Widgets stable releases. When a user wanted to know what changed in, say, @webex/cc-widgets between 1.28.0-next.5 and 1.28.2-next.7, they had to manually open each version log file and scan through commits — across multiple intermediate stable versions (1.28.0, 1.28.1, 1.28.2) — with no tooling to aggregate them.
by making the following changes
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
Applies position-based rules for which pre-release entries to include per stable:
start — from the user's base pre-release (e.g. 1.28.0-next.5) through all remaining pre-releases of that stable
middle — skips the exact stable entry (e.g. 1.28.1); collects ALL pre-releases of that intermediate stable
end — from next.1 up to the user's target pre-release (e.g. 1.28.2-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 between the two selected versions.
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.