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

Db Connection implementation: handle_execute + prepare + bind #3

Merged
merged 3 commits into from
Feb 20, 2021

Conversation

dmitriid
Copy link
Contributor

@dmitriid dmitriid commented Feb 20, 2021

This builds on #2.

Relevant commits:

  • 1242c1e handle_execute implementation. Includes prepare and bind
  • 404ce4d and 99f4f09 are minor code fixes to handle this

Note: everything is as barebones as possible, just to check that everything is working. So, no funny majick around bind, for example :) See inline comments for TODOs

@warmwaffles
Copy link
Member

@dmitriid could you do a rebase or update your branch based on what is merged in?

@dmitriid dmitriid force-pushed the db-connection-continue branch from 8e39515 to 1242c1e Compare February 20, 2021 21:47
@dmitriid
Copy link
Contributor Author

@warmwaffles Did a quick rebase.

I've seen your commits into #1 but haven't looked at them in detail 👍 It looks like we're doing more-or-less the same thing. You're just doing it faster :D

@warmwaffles
Copy link
Member

@dmitriid I've just been cobbling together stuff and refactoring. There is just so much extra cruft in all of this.

I'm going to go the route you are taking and make these smaller chunks.

@dmitriid
Copy link
Contributor Author

@warmwaffles That's so true. This was the case when I tried to go in guns a-blazing in https://github.com/dmitriid/sqelect/tree/update-attempt-the-first

It does become weird when you go into the actual ecto adapter. Suddenly you feel like you need 100 things at once (encoding, decoding, oh my). But may be taking things chunk by chunk will work.

@warmwaffles
Copy link
Member

It does become weird when you go into the actual ecto adapter. Suddenly you feel like you need 100 things at once (encoding, decoding, oh my). But may be taking things chunk by chunk will work.

Yea that was really confusing. I am trying to figure out parts we can lift out and test appropriately.

Though as I just worked through trying to make tests pass I was able to decipher what the intentions were.

@warmwaffles warmwaffles merged commit cec51fa into elixir-sqlite:main Feb 20, 2021
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.

2 participants