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

Add support for sqlx NamedStmt #26

Merged
merged 1 commit into from
Sep 4, 2023
Merged

Add support for sqlx NamedStmt #26

merged 1 commit into from
Sep 4, 2023

Conversation

Thiht
Copy link
Contributor

@Thiht Thiht commented Aug 25, 2023

Additionally to Stmt and Rows, sqlx exposes NamedStmt
As for Stmt, they should also be closed to avoid pool exhaustion and potential leaks

ryanrolds
ryanrolds previously approved these changes Aug 27, 2023
Copy link
Owner

@ryanrolds ryanrolds left a comment

Choose a reason for hiding this comment

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

Thank you.

I intend to merge this after #25 is merged.

@ryanrolds
Copy link
Owner

There is some conflicts. Once those are resolved I will merge.

@ryanrolds
Copy link
Owner

ryanrolds commented Aug 27, 2023

I've enabled CI on PRs in main. Don't be surprised if you get lint errors or failing tests. If you need help resolving CI failures, let me know.

@Thiht
Copy link
Contributor Author

Thiht commented Aug 28, 2023

There is some conflicts. Once those are resolved I will merge.

@ryanrolds Updated!

@Thiht
Copy link
Contributor Author

Thiht commented Aug 29, 2023

Sorry, just noticed I didn't stage my changes 😅

Updated for real this time.

Lint/test/build OK in local, I don't think I can trigger the CI myself

@Thiht Thiht requested a review from ryanrolds August 30, 2023 12:52
@ryanrolds
Copy link
Owner

Sorry. I wasn't aware that GH wouldn't run CI automatically on this PR.

@ryanrolds ryanrolds merged commit f6bc14a into ryanrolds:main Sep 4, 2023
1 check passed
@Thiht
Copy link
Contributor Author

Thiht commented Sep 5, 2023

No problem, thanks for the merge!

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

2 participants