Skip to content

Add ability to prepend query comments #5289

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

Merged
merged 2 commits into from
Apr 14, 2023

Conversation

francois2metz
Copy link
Contributor

@francois2metz francois2metz commented Aug 4, 2022

This is a rebased and updated version of #2815 that fixes #1982.

About parameter binding, I did tried to escape them, but I doesn't works for mysql. So I forbidden any ?.

@OlivierCavadenti OlivierCavadenti self-requested a review August 4, 2022 12:10
@coveralls
Copy link

coveralls commented Aug 4, 2022

Coverage Status

Coverage: 92.836% (+0.5%) from 92.334% when pulling 32a4425 on francois2metz:feature/query_comment into f7ccde8 on knex:master.

@francois2metz
Copy link
Contributor Author

I fixed a bug with cockroachdb as ids are not numeric. It should be fine now. I'll squash commit later.

@kibertoad
Copy link
Collaborator

@francois2metz Can you please send documentation PR to https://github.com/knex/documentation?

@francois2metz
Copy link
Contributor Author

@kibertoad Hi! I created the documentation PR: knex/documentation#450

Copy link
Collaborator

@OlivierCavadenti OlivierCavadenti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok for me ! Thanks and sorry for the delay

@francois2metz francois2metz force-pushed the feature/query_comment branch 3 times, most recently from c4b5b2f to 3890007 Compare September 23, 2022 08:13
Co-authored-by: François de Metz <francois@2metz.fr>
Copy link

@JMHenri JMHenri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OlivierCavadenti

My team would love to have this PR in (with postgres added). If it's possible to get this in, we can write up a pr for adding in postgres.

Copy link
Collaborator

@OlivierCavadenti OlivierCavadenti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update types here and I will merge it:
https://github.com/knex/knex/blob/master/types/index.d.ts

@OlivierCavadenti OlivierCavadenti merged commit 704d12e into knex:master Apr 14, 2023
@francois2metz francois2metz deleted the feature/query_comment branch April 14, 2023 10:30
@lindakung
Copy link

Very excited for this! Any ETA on when a new version will be released?

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.

Query comment
7 participants