-
Notifications
You must be signed in to change notification settings - Fork 39
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
Update dependencies #1473
Update dependencies #1473
Conversation
Dependencies with new minor versions updated via npm update. commander updated to new major version safely. Other dependencies with new major versions are not updated due to the nature of their respective breaking changes.
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.
I tested this out and things seem to be working well. I have one suggestion and one question:
- We have a
DEPENDENCY-NOTES.md
file where we've documented dependencies that we can't updated. It sounds like some fromnpm outdated
would fit in that document. If so, can you add them? - This is just a question and it is very possible it isn't necessary at all, but the last time we updated dependencies, we updated the
package.json
entry for them, even if it was only a minor or patch version update. Is it worth doing that again as well? Or are we planning to just utilize thenpm-shrinkwrap.json
file for this?
Thank you for the reminder about |
Update DEPENDENCY-NOTES.md to reflect that the non-updated dependencies have been checked recently.
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.
Thanks for updating package.json
!
When I run npm outdated
, there's a few packages (I think all ESLint related?) that have major upgrades available. Do we want to hold off on upgrading ESLint for now? I've had success with upgrading to the new major version of ESLint in a different repo, but if there's a reason we don't want to update it here, we probably want to add it to the DEPENDENCY-NOTES.md
file.
Other minor updates based on releases in the last few days.
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.
Thanks Mint! I'm sorry to ask about something else, but now when I run npm install
, I get warnings coming from the @typescript-eslint
packages requiring Node to be >= 18.18.0. Do you mind updating the .tool-versions
file to use a later 18.x version? I'm fine with either just using 18.18.0, using whatever version you have locally, or using the latest, which is 18.20.3.
This does remind me that I guess we could update to Node 20 at some point, but that could be out of scope for this task.
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.
Thanks @mint-thompson and @jafeltra. This looks good to me. It installs without a problem and everything runs well. All outdated dependencies are documented.
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.
Looks good!
Completes task CIMPL-1263.
Dependencies with new minor versions updated via npm update. commander updated to new major version safely. Other dependencies with new major versions are not updated due to the nature of their respective breaking changes (mostly, removal of CommonJS support).