-
Notifications
You must be signed in to change notification settings - Fork 3
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
nsc-events-nestjs-12-141-upgrade-packages-and-node-LTS #143
Conversation
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.
Everything seems to be working fine based on initial tests.
Everything seems to be working fine based on initial tests. The PR touches key packages like @nestjs/cli and @typescript-eslint/parser, which should improve performance and security. The upgrades are looking great with no obvious issues or conflicts. It might be worth testing on a couple of different Node.js versions to ensure the upgrades work across environments, especially for the critical dependencies accordingly. Some manual testing would also help cover any missed cases. I found a useful link on : Best Practices and Benefits for Maintaining Up-to-Date Dependencies |
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, the testing went well on my end, I also tested in conjunction with the other branch you mentioned:
I did encounter a console.warn for the API key (but it's probably on my end maybe):
only minor change I would request is getting rid of the karats ^
in your package.json since were trying to enforce that from what @bcko said:
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 Great, Testing the pages went the same way but the caret removal is much appreciated, Sorry for the late check on your new changes, Thanks @bennettsf
Resolves #141 (continuation of @Alzavio 's issue)
This PR dependencies/packages as well as node to 20.xx LTS
I've done some testing to make sure all current features are working properly and there are no dependency conflicts with the upgrades.
Also ran our current tests (npm run test) :
I'm going to spend more time testing this weekend to make sure I didn't miss anything. If you have any recommendations or concerns please drop a comment.
It's recommended to test this in conjunction with the frontend dependency upgrades PR (located here: SeattleColleges/nsc-events-nextjs#560) although probably not necessary.