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

Feat: Removed sql-ts #3

Merged
merged 2 commits into from Apr 20, 2021
Merged

Feat: Removed sql-ts #3

merged 2 commits into from Apr 20, 2021

Conversation

Jytesh
Copy link
Collaborator

@Jytesh Jytesh commented Apr 20, 2021

queries are built manually by the keyv-sql package instead of depending on external packages

Signed-off-by: Jytesh 44925963+Jytesh@users.noreply.github.com

Fixes #2, Fixes https://github.com/lukechilds/keyv-sql/issues/26

sql-ts is not compatible with node v10.

queries are built manually by the keyv-sql package instead of depending on external packages

Signed-off-by: Jytesh <44925963+Jytesh@users.noreply.github.com>
@Jytesh Jytesh added the enhancement New feature or request label Apr 20, 2021
@Jytesh Jytesh requested a review from Kikobeats April 20, 2021 14:18
@Jytesh Jytesh self-assigned this Apr 20, 2021
@Kikobeats
Copy link
Member

@Jytesh do you think makes sense to target LTS (Node14) and current (Node15) on GitHub Action tests?

Just to be sure the change works for both

@Jytesh
Copy link
Collaborator Author

Jytesh commented Apr 20, 2021

I was thinking we can add all even versions from 10 - 15 ( including 15 )

@Jytesh
Copy link
Collaborator Author

Jytesh commented Apr 20, 2021

Since we shouldn't drop support for 10 just yet

@Kikobeats
Copy link
Member

if the idea is going to ship this as a major version, then it's a good opportunity to drop old node versions 🙂

I think node 12 could be the oldest version supported.

Check this for adding the matrix version of Node.js for testing:

https://docs.github.com/en/actions/guides/building-and-testing-nodejs

@Jytesh
Copy link
Collaborator Author

Jytesh commented Apr 20, 2021

Aight 12 sounds good.

@Jytesh
Copy link
Collaborator Author

Jytesh commented Apr 20, 2021

12, 14 and 15 to be tested then?

Signed-off-by: Jytesh <44925963+Jytesh@users.noreply.github.com>
@Kikobeats
Copy link
Member

yes!

@Kikobeats
Copy link
Member

awesome 🎉

@Jytesh Jytesh merged commit 669fe06 into master Apr 20, 2021
@Jytesh Jytesh deleted the sql-patch-1 branch April 20, 2021 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade sql-ts to knex
2 participants