Add Sequel plugin #12

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@jeremyevans
Contributor

This plugin is based on the data_mapper plugin, modified to use
the Sequel API. Sequel natively supports most features of
orm_adapter, only the association= method needs to be emulated.

@jeremyevans jeremyevans Add Sequel plugin
This plugin is based on the data_mapper plugin, modified to use
the Sequel API.  Sequel natively supports most features of
orm_adapter, only the association= method needs to be emulated.
1d33558
@ianwhite
Owner
ianwhite commented Apr 6, 2012

Hi Jeremy,

I'm not sure that orm_adapter should patch Sequel to make it work. I'd be surprised by that behaviour in an adapter plugin. Also, the pull request contains no tests.

But, it's very easy to make your own gem orm_adapter-sequel, or similar.

Cheers,
Ian

@ianwhite ianwhite closed this Apr 6, 2012
@ianwhite ianwhite reopened this Apr 6, 2012
@ianwhite
Owner
ianwhite commented Apr 6, 2012

My sincere apologies, I didn't see the specs for some reason, re-evaluating...

@ianwhite
Owner
ianwhite commented Apr 6, 2012

Hi Jeremy,

Really sorry about that, specs are all good. Also, the adapter does't patch Sequel any more than any of the other adapters do.

So, I'm happy with this patch. José, any thoughts?

Cheers,
Ian

@ianwhite
Owner
ianwhite commented Apr 6, 2012

Hi Jeremy,
Is there a good reason not to use the existing orm_adapter-sequel gem?
As if you couldn't guess - I'm not a Sequel user :)
Cheers,
Ian

@jeremyevans
Contributor

Sorry, I wasn't aware of orm_adapter-sequel when I wrote the support. Looking at it, it seems more complete than what I wrote, which basically is the minimum needed to pass the specs (I don't use orm_adapter myself). I'll close this now. Thanks for reviewing. :)

@jeremyevans jeremyevans closed this Apr 6, 2012
@ianwhite
Owner
ianwhite commented Apr 6, 2012

Hi Jeremy,

No worries, sorry about all the noise (not enough coffee before OSS...) and thanks for the other patch btw.

Cheers,
Ian

@ujifgc ujifgc referenced this pull request in padrino/padrino-framework Jun 14, 2014
Open

Admin internal API interface #1494

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment