-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
fix(release): fix issues with maintenance and active lts #7979
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?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
This PR refines how Long-Term Support (LTS) statuses are labeled and consumed throughout the site, introducing distinct “Active LTS” and “Maintenance LTS” labels and migrating from status string checks to an isLts
flag.
- Expanded the set of release statuses to include separate active and maintenance LTS labels
- Updated the release status generator and type definitions to emit and handle the new labels
- Refactored UI components and data routes to use
isLts
and display the updated labels
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
apps/site/util/download/constants.json | Added “Active LTS” and “Maintenance LTS” to the supported release list |
apps/site/types/releases.ts | Replaced 'LTS' /'Maintenance' with 'Active LTS' /'Maintenance LTS' |
apps/site/next-data/generators/releaseData.mjs | Updated status returns in getNodeReleaseStatus and isLts mapping |
apps/site/app/[locale]/next-data/api-data/route.ts | Switched from .status === 'LTS' to .isLts when selecting version |
apps/site/components/Downloads/Release/VersionDropdown.tsx | Generalized LTS detection to endsWith('LTS') for dropdown labels |
Comments suppressed due to low confidence (1)
apps/site/components/Downloads/Release/VersionDropdown.tsx:15
- [nitpick] Collapsing both 'Active LTS' and 'Maintenance LTS' into a generic '(LTS)' suffix loses the distinction between the two. Consider branching so that active releases display
(Active LTS)
and maintenance releases(Maintenance LTS)
.
if (status.endsWith('LTS')) {
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #7979 +/- ##
==========================================
- Coverage 73.10% 73.08% -0.03%
==========================================
Files 95 95
Lines 8355 8355
Branches 219 219
==========================================
- Hits 6108 6106 -2
- Misses 2246 2248 +2
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Copilot <[email protected]> Signed-off-by: Aviv Keller <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Aviv Keller <[email protected]>
@avivkeller I assume your closures of #7966 and #7965 allude to this? i didnt see code or a PR attached to them? |
I don't think they were fully resolved. @bmuenzenmeyer was also questioning the LTS schedule generation itself to get updated and other things... But not sure if they are related. |
I think those concerns are in nodejs/Release#1111? |
We can also re-open the previous issue if I made the wrong call. |
oh, apologies, i wasnt aware that that PR handled my issues (as they were opened while reviewing it |
All fine, I noticed those are not totally related, I think this PR is good and appreciate your contribution here ✨ |
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.
SGTM. I hope this PR covers all paths and doesn't accidentally break something haha (I bet TS covers that, but who knows)
There are many times where we reference the
'LTS'
status in an attempt to get an Long-Term Support version. However, the'LTS'
status actually refers to the Active Long-Term Support version.This PR makes the Maintenance vs Active LTS labels more distinct (as to not cause confusion), and replaces instances of checking
'LTS'
withisLts
.