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

Make RethinkORM::Base a module #2

Open
watzon opened this issue Feb 21, 2020 · 3 comments
Open

Make RethinkORM::Base a module #2

watzon opened this issue Feb 21, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@watzon
Copy link

watzon commented Feb 21, 2020

I'm going to echo the same comment I made with Granite. It would be really nice if RethinkORM::Base were a module rather than an abstract class. The main two reasons I can think of as to why this would be good are:

  1. You want this to be an optional dependency, so in a class you want to override some class definitions and include RethinkORM::Base (my case)
  2. Your model class needs to extend another type. Having to extend this will keep you from doing that.

This would obviously be a breaking change, but maybe it doesn't have to be. If the logic in RethinkORM::Base were moved into a module with a different name, and then that module were included into RethinkORM::Base we could probably have our cake and eat it too.

Thoughts?

@caspiano caspiano added the enhancement New feature or request label Feb 23, 2020
@caspiano
Copy link
Contributor

Hey, thanks for the suggestion!
I agree. This library could be beneficially refactored into a module to allow for looser coupling however there's quite a bit of dependency on active-model.

I'll have a think about this over the next few days and see if I can come up with anything better.

@watzon
Copy link
Author

watzon commented Feb 23, 2020

Awesome! It would be a lifesaver for me if it would be possible. Right now there aren't many ORMs that use a module. Most use the abstract class approach.

@caspiano
Copy link
Contributor

caspiano commented Mar 4, 2020

Unfortunately, I haven't had time to work on this 😕
My current priority with this project is to add connection pooling, as well as a few other touch-ups. But the decoupling is definitely on the agenda.

Feel free to open up a PR if you're up for it 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants