Skip to content
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

Upgrade knex to ^2 #142

Merged
merged 5 commits into from Sep 18, 2023
Merged

Upgrade knex to ^2 #142

merged 5 commits into from Sep 18, 2023

Conversation

thyen
Copy link

@thyen thyen commented Sep 12, 2023

This PR:

  • Upgrades node runtime to 18.17
  • Updates the knex dependency to ^2.5.1
    • This is a BREAKING CHANGE matching the breaking change version from knex

@joelmukuthu
Copy link
Member

Thanks for the PR Thyen! I need to fix up some things here before I can deploy a new version. In the meantime, do you know in what version knex changed the paths? I'd like to know what other knex versions could potentially be supported in the peer-dependency range.

@thyen
Copy link
Author

thyen commented Sep 13, 2023

File structured change PR knex/knex#4178
Looks like it was on version 0.95.0

@thyen
Copy link
Author

thyen commented Sep 13, 2023

Updated the PR with appropriate peer dependency range

@joelmukuthu
Copy link
Member

I've update dev deps and gotten tests to run again, would you please rebase your branch Thyen? You can skip commit c30640c that's already applied on master. In the commit that upgrades Knex, you'll also need to update this path in the tests:

require('knex/lib/util/import-file.js');

I think the new path is knex/lib/migrations/util/import-file.js.

@thyen
Copy link
Author

thyen commented Sep 17, 2023

Donezo! Rebasted with your master changes and updates the .spec.js import reference

@joelmukuthu
Copy link
Member

This path also needs an update

const QueryBuilder = require('knex/lib/query/builder');

And then run npm run lint:fix to fix the formatting error :)

@thyen
Copy link
Author

thyen commented Sep 18, 2023

Applied the lint fixes, updated the node version on Dockerfile and ran the tests locally to confirm it all passes, I think we're good now

@coveralls
Copy link

Coverage Status

coverage: 100.0%. remained the same when pulling 1834d79 on thyen:master into edad6a4 on unexpectedjs:master.

@joelmukuthu
Copy link
Member

Perfect, thanks Thyen!

@joelmukuthu joelmukuthu merged commit ee9f4fa into unexpectedjs:master Sep 18, 2023
6 checks passed
@joelmukuthu
Copy link
Member

Released in v5.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants