Skip to content

Add support for Sequel ORM #9

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

Merged
merged 1 commit into from
May 1, 2014
Merged

Conversation

kamui
Copy link
Contributor

@kamui kamui commented May 1, 2014

The sequel orm has it's own #pagination method. will_paginate doesn't override or wrap this method, it instead just uses the built in sequel pagination method. However, sequel's #paginate has a different method signature, instead of a hash or named parameters, it looks like this:

   def paginate(page_no, page_size, record_count=nil)
     # ...
   end

I couldn't think of a cleaner way to support Sequel, so what it does is check to see if collections inherits from Sequel::Dataset and if it does, it'll call #paginate with Sequel's method signature. If you have a better way to implement this, let me know and I'll do my best to modify this PR.

@kamui kamui changed the title Add support for Sequel and will_paginate Add support for Sequel May 1, 2014
@kamui kamui changed the title Add support for Sequel Add support for Sequel ORM May 1, 2014
@kamui
Copy link
Contributor Author

kamui commented May 1, 2014

BTW will_paginate supports sequel, but mislav recommends when you call paginate, you use sequel's paginate method signature. So, that's why this bug exists when you use sequel and will_paginate in api-pagination.

@davidcelis
Copy link
Owner

Hey, this looks good to me. Thanks for submitting a fix! Would you mind squashing before I merge?

@kamui
Copy link
Contributor Author

kamui commented May 1, 2014

There you go.

@davidcelis
Copy link
Owner

Thanks! Looks like the build issues are related to travis-ci/travis-ci#2220

davidcelis added a commit that referenced this pull request May 1, 2014
Add support for Sequel ORM
@davidcelis davidcelis merged commit 790eb15 into davidcelis:master May 1, 2014
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