elixir-sqlite / exqlite

An SQLite3 driver for Elixir
https://hexdocs.pm/exqlite
MIT License
217 stars 48 forks source link

Upstreaming the ecto-related parts #113

Closed kevinlang closed 3 years ago

kevinlang commented 3 years ago

I was going to start a new conversation in the elixir-ecto Google group to see what the path forward is for upstreaming most of the ecto related stuff into ecto_sql, now that nearly all of the integration tests are onboarded and stable.

In that scenario, this exqlite repo would mainly contain the core non-ecto driver parts, like the db_connection stuff and the NIF.

Does that sound good, @warmwaffles ? Any reservations or thoughts?

warmwaffles commented 3 years ago

That's totally okay with me. The only bit that would be staying here would be the db_connection implementation.

kevinlang commented 3 years ago

Cool, here is the thread:

https://groups.google.com/g/elixir-ecto/c/Y27uRzCHSRE

lawik commented 3 years ago

I saw that. I hope that isn't too discouraging. I don't think it is really needed to be in ecto_sql to be a good option. Unless I'm missing something.

kevinlang commented 3 years ago

Despite this, I still think it is a good idea to split out the ecto adapter from the actual exqlite driver as we approach 1.0, call it something like ecto_sqlite3. It would be nice for the two to be organized under the same Github organization, perhaps we could merge into the elixir-sqlite org. :man_shrugging:

warmwaffles commented 3 years ago

So I was thinking about pulling the adapter into exqlite_ecto. Possibly keep them both in this repository for now similar to Nx.

warmwaffles commented 3 years ago

What are your thoughts on keeping them both in the same repo but under different directories?

kevinlang commented 3 years ago

I think Nx uses a single repo similar for the way we do it: for ease of rapid iteration until they hit stable. From the README:

they will be extracted to their own repository in the future. 

But I think having them as separate projects in the same repo is fine for now. I do think there is always a tradeoff between keeping things well-separated and maintenance/developer ergonomics, and keeping them in the same repo but be separate Mix projects seems to be a nice middle ground.

warmwaffles commented 3 years ago

When do you think we should split the adapter? My plan is to open another repo, push this repo into it so we keep the history and then delete the parts that don't belong.

kevinlang commented 3 years ago

That sounds like a good plan. I think we can do it whenever.

For naming, some things to consider:

What do you think?

warmwaffles commented 3 years ago

I think I am okay with that plan. We should probably start renaming the adapter now and preparing for that.

warmwaffles commented 3 years ago

The biggest problem is, people are using this adapter now and it will break their projects if they aren't notified / we don't get a proper warning system in place.

axelson commented 3 years ago

The biggest problem is, people are using this adapter now and it will break their projects if they aren't notified / we don't get a proper warning system in place.

I wouldn't think you need to worry about that too much. I doubt there's many people who have integrated exqlite since it is such a new project and well known to be under heavy development, so breaking changes at this stage should be expected. In my opinion, a simple note in the new changelog (https://github.com/warmwaffles/exqlite/issues/114) would be sufficient.

kevinlang commented 3 years ago

I started looking into setting up a ecto_sqlite3 repo.

Any reservations around creating an Organization or merging our efforts into the elixir-sqlite org? In either case, I created a simple logo.

logo

warmwaffles commented 3 years ago

Yea that's fine, we just need access to that org https://github.com/elixir-sqlite/

kevinlang commented 3 years ago

@ConnorRigby, what say you?

warmwaffles commented 3 years ago

@kevinlang can you shoot me an email at warmwaffles@gmail.com need to co-ordinate how to get you package push permissions and what not.

kevinlang commented 3 years ago

I created an account there kevinlang, which you should be able to use to add me via mix hex.owner add

Here is the repo of ecto_sqlite3 (for now, until the Org is configured): https://github.com/kevinlang/ecto_sqlite3

And here is the Hex package: https://hex.pm/packages/ecto_sqlite3

I added you @warmwaffles as another owner.

warmwaffles commented 3 years ago

@kevinlang okay I'm going to open a PR here to add a changelog and remove the adapter from this repo.

warmwaffles commented 3 years ago

Also @kevinlang https://twitter.com/pressy4pie/status/1370406636066467840 @ConnorRigby will get us added to that org tomorrow.

kevinlang commented 3 years ago

I transfered the ecto_sqlite3 repo over.

https://github.com/elixir-sqlite/ecto_sqlite3

warmwaffles commented 3 years ago

Just did the same here too

kevinlang commented 3 years ago

Cool. @ConnorRigby do you mind updating the Org logo to what I linked above? I can't since I don't have Owner permissions.

I'll close this one out for now.