-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[cli] Allows warnings for ls --update-required
to show for deprecated versions
#12856
Conversation
🦋 Changeset detectedLatest commit: 226f27e 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 |
nodeVersion.discontinueDate && | ||
nodeVersion.discontinueDate.valueOf() > Date.now() | ||
nodeVersion.deprecationDate && | ||
nodeVersion.deprecationDate.valueOf() > Date.now() |
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.
suggestion (blocking): check both dates
I think we want to check for either date being in the past. Or, we should add deprecationDate
to all currently discontinued versions.
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.
Looking more at this feature as it currently exists and it makes no sense. It only works as a warning after your projects already don't run? Gonna rethink this entire thing.
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.
Checking discontinueDate
means that you already can't deploy, sure. A user still wants to get a list of those, though.
Checking deprecationDate
means that you can know ahead of time.
I think we want to check both.
226f27e
to
60cd7ea
Compare
Closes #12856. The existing logic assumes that if a `discontinueDate` has been set for a given version of node, that version of node must be deprecated. It was also easy to mistake the previous code as displaying the warning only _after_ the discontinue date, which it never did. This PR: - Clarifies the language communicating the deprecation to users. It's not that the node version _will be_ deprecated. The node version is _already_ deprecated and is at risk of being discontinued. - Adds an identifier to the logic checking for a deprecated node version, making it clear that the date check is looking into the future. - Adds a comment clarifying the logic further, and documents the assumption that a `discontinueDate` means that a node version is deprecated. - Adds a further comment noting that if this assumption is invalid, additional logic may be required. Specifically, in the case where we have deprecated a node version, but don't yet know its discontinue date.
Currently we only show a warning if a runtime version's
discontinueDate
has passed, but customers should be able to run this before that date to see what they need to change. Setting the date for the soon-to-be removed node@16 to the date from the changelog