-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Updated knex and postgres dependencies #2862
Conversation
🦋 Changeset is good to goLatest commit: fc9fb29 We got this. This PR includes changesets to release 8 packages
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 |
I think this may be a major breaking change. We need to see if any of the breaking changes to |
@timleslie The one called out in the original issue is enough to justify a breaking change in my opinion:
|
Co-authored-by: Mike <mike@madebymike.com.au>
@timleslie will leave this to you to go through in detail. My 2c below --
Based on the
Change log. Looks quite safe. The only two things that jumped out at me were...
And of course you'd only hit them if you were using Keystones knex instance for some pretty specific stuff. |
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 think if we explicitly call out the versions being upgraded then we can push this one through 👍
Co-authored-by: Tim Leslie <tim.leslie@gmail.com>
Committed your suggestion. |
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.
👍
Finally! |
Should resolve #2849 as requested.