-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add daysUntilEOL and toJSON #826
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
Conversation
🦋 Changeset detectedLatest commit: 39d7b0a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe changes introduce a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #826 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 46 55 +9
Branches 16 23 +7
=========================================
+ Hits 46 55 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
b4e370b to
5c30879
Compare
Reverting local changes and closing task as PR #826 supersedes this work.
Greptile SummaryAdded two developer experience improvements to
The implementation is clean, well-tested, and follows the existing code patterns. The changeset correctly marks this as a minor version bump. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant getVersion
participant NodeVersion
participant Date
participant EOL_DATES
User->>getVersion: Call getVersion()
getVersion->>process: Read versions.node and release.lts
getVersion->>EOL_DATES: Lookup major version
EOL_DATES-->>getVersion: Return EOL date string or undefined
alt EOL date exists
getVersion->>Date: new Date(eolString)
Date-->>getVersion: eolDate object
getVersion->>Date: Date.now()
Date-->>getVersion: Current timestamp
getVersion->>getVersion: Calculate (eolDate - now) / ms_per_day
getVersion->>Math: Math.ceil(daysDifference)
Math-->>getVersion: daysUntilEOL (positive/negative)
else No EOL date
getVersion->>getVersion: Set daysUntilEOL = undefined
end
getVersion->>getVersion: Build dataProps object
Note over getVersion: dataProps includes: original, short, long,<br/>major, minor, build, isLTS, ltsName,<br/>isEOL, eolDate, daysUntilEOL
getVersion->>NodeVersion: Create NodeVersion object
Note over NodeVersion: Spreads dataProps + adds methods:<br/>isAtLeast, is, isAbove, isBelow,<br/>isAtMost, toString, toJSON
getVersion-->>User: Return NodeVersion
alt User calls JSON.stringify
User->>NodeVersion: JSON.stringify(version)
NodeVersion->>NodeVersion: toJSON() called automatically
NodeVersion-->>User: dataProps only (no methods)
end
alt User accesses daysUntilEOL
User->>NodeVersion: version.daysUntilEOL
NodeVersion-->>User: number (pos/neg) or undefined
end
|
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.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/index.ts">
<violation number="1" location="src/index.ts:51">
P2: `Math.ceil` returns 0 for negative values between 0 and -1, so `daysUntilEOL` won’t be negative for up to 24 hours after EOL even though the version is already past EOL. Use a calculation that yields a negative value immediately after the date passes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
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.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/index.ts">
<violation number="1" location="src/index.ts:52">
P2: Using Math.floor makes `daysUntilEOL` return 0 when less than 1 day remains before EOL, which contradicts the expectation that the value is positive for any pre‑EOL date. Use sign-aware rounding so values stay >0 before EOL and <0 after.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Summary
daysUntilEOLproperty (days until EOL, negative if past)toJSON()method for clean JSON serializationExtracted from bot PRs #797 and #814, implemented cleanly.
Closes
Supersedes all 28 open bot PRs from
google-labs-jules.Summary by cubic
Adds daysUntilEOL to NodeVersion and a toJSON method for clean, data-only serialization. Improves EOL awareness and ensures JSON.stringify excludes methods.
Written for commit 39d7b0a. Summary will update on new commits.
Summary by CodeRabbit
New Features
daysUntilEOLproperty to track days until a Node version reaches end-of-life.toJSON()method to enable JSON serialization of version data.NodeVersionJSONtype for type-safe JSON representation.Tests
✏️ Tip: You can customize this high-level summary in your review settings.