-
Notifications
You must be signed in to change notification settings - Fork 43
update to node 24 packages #1366
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: master
Are you sure you want to change the base?
Conversation
| await mongoStore.collection.drop() | ||
| await mongoStore.close() | ||
| if (mongoStore) { | ||
| await mongoStore.collection.drop() |
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 found during some local testing that the before() function didn't always complete before after() which would mean the variable mongoStore was still undefined and this line would throw.
pombredanne
left a comment
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. Looking great.
|
@jeffmendoza and @pombredanne You were right, the CI check was failing because of bad formatting in |
Hi all, I'm Brett and I'm a software engineer at Microsoft. We use ClearlyDefined a lot internally and I was asked by my management to update some things in this repo. I figured I should start with something small and hopefully easy to review. Some of the node-adjacent packages used referenced either node 20 or outdated Docker images so this is an attempt to rectify that.
My plan for future PRs is to update various dependencies as required along with improving test coverage where possible.
I'm open to any and all feedback and if you'd like the feedback to be private, you can email me directly at
brettfo@microsoft.com.