-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: support geometry #3449
base: main
Are you sure you want to change the base?
feat: support geometry #3449
Conversation
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.
Hi, I have commented a few things inline ^^
Overall, looks quite similar to PgCube
Ty for taking the time to review! |
Just noticed a bunch of changes to As this was based on cube.rs before changes, I will backport it to these changes for consistency. |
2d7b88d
to
d16c98e
Compare
I agree it is pretty large, appreciate your patience with reviewing it. @abonander if you prefer I can split it up into chunks to review instead. Realise my cube work may have left you with some headache. |
Is that still alive? I would really need it for a project. Thanks for the work ! |
The PR has been raised a month ago and is a 2k lines humongous PR. You can always patch your cargo dependencys (docs: https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html), but that comes (obviously) at some risk |
I'll break up the size and do a component at a time for reviewers, will carry over feedback that's been received so far. |
That would be great, thank you. 2k lines is a lot to go through with the time I have to spend on SQLx. |
fixes #166
If we're happy with the way I'm implementing point (and maybe line/lseg),
I will continue with other types.(Got carried away a little, fun problem. Any changes requested with one type I'm more than happy to copy it across to others)