-
Notifications
You must be signed in to change notification settings - Fork 68
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
support pgx v5 #94
support pgx v5 #94
Conversation
this requires v2 of scany library due to breaking changes
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/georgysavva/scany/dbscan" | ||
"github.com/georgysavva/scany/v2/dbscan" |
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.
Aren't you missing this directory from the patch?
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.
not really, v2 is the version of the module, so it doesn't need to have a directory
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.
Agree with @vadimi
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.
Thanks, I am still struggling with the "new" module system :-)
Codecov Report
@@ Coverage Diff @@
## master #94 +/- ##
=======================================
Coverage 82.81% 82.81%
=======================================
Files 5 5
Lines 512 512
=======================================
Hits 424 424
Misses 73 73
Partials 15 15
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@vadimi Thank you for your work. It looks great! I think we also need to update the readme file to use the new module name |
@georgysavva thanks, that makes sense, updated the readme. I also noticed golangci lint is failing, so I updated it to latest to see if it helps. |
Thanks for this, eagerly awaiting this to be released :-) |
Looking forward to seeing this released - been on a big OpenTelemetry push recently, so will be nice to be able to use the new support in pgx5 for that. In the mean time, I've been using a replace directive in my
|
Hands up who's reason is OpenTelemetry too! :-) Thanks, nice trick actually. Had no idea it works remotely too. |
@vadimi Thanks for updating the readme! Yes, I also noticed that golangci-lint is broken. I fixed it in the master, along with other GitHub action bumps. So I just merged the changes into your branch. I also fixed a few leftovers for the old version paths. Now the PR is ready. Thank you so much for taking on it! I am going to proceed with merging it and releasing the v2 tag. |
Here everyone. I created a pre-release for Please test it out a little bit, and if everything is fine, I will make the stable release. |
Thank you for this PR, but you missed something - import |
It looks like in pgx v5 github.com/jackc/pgtype package needs to be replaced with github.com/jackc/pgx/v5/pgtype https://github.com/jackc/pgx/blob/f803c790d0999e9028cbffefad9df02e4aff913a/CHANGELOG.md#merged-packages as mentioned by @kohenkatz in PR #94
since pgx v5 contains breaking changes (supports go 1.18+ only) this package also needs to have a version bump - v2
should fix #88