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

Fix Pagination plugin #1258 #1272 #1561

Merged
merged 3 commits into from Jan 17, 2018
Merged

Conversation

@pmettraux
Copy link

@pmettraux pmettraux commented May 1, 2017

based on vellotis work. see https://github.com/tgriesser/bookshelf/pull/1272/files

@vellotis I hope you don't mind I took what you did and based it on the current version of bookshelf, you deserve all the credits

@pmettraux
Copy link
Author

@pmettraux pmettraux commented May 1, 2017

Shit tests fail, my bad

Patrick mettraux
@pmettraux
Copy link
Author

@pmettraux pmettraux commented May 1, 2017

Ok fixed

@vellotis
Copy link
Contributor

@vellotis vellotis commented May 1, 2017

@pmettraux Great work!

I forgot that not everybody has a long weekend. 1st of May is a holiday in wide range of European countries. I just wanted to start rebasing.

Sadly as this is still my implementation. I am not willing to merge it. @kirrg001 please review and merge the PR.

@Playrom
Copy link
Contributor

@Playrom Playrom commented Jun 29, 2017

ok guys, I'm here for you! what's the state of this PR?

@Playrom
Copy link
Contributor

@Playrom Playrom commented Jul 21, 2017

can you guys provide with a comment a PR description as in the new template PULL-REQUEST-TEMPLATE.md?

I'm willing to merge this PR but I want to read a clear description of the effects and behaviour

@vellotis @pmettraux

@Playrom Playrom added the plugin label Jul 21, 2017
@jadengore
Copy link
Contributor

@jadengore jadengore commented Jul 23, 2017

@pmettraux Willing to review this PR, please respond to thread before I begin

@jadengore jadengore self-assigned this Jul 23, 2017
@pmettraux
Copy link
Author

@pmettraux pmettraux commented Jul 23, 2017

I have been working on a fork since nobody was answering. Now obviously conflicts appeared. The entire discussion is here : #1272

@Playrom
Copy link
Contributor

@Playrom Playrom commented Jul 23, 2017

@jadengore if you can do us the favour to review this whole fix it would be awesome 👍

@jadengore jadengore self-requested a review Jul 23, 2017
@jadengore
Copy link
Contributor

@jadengore jadengore commented Jul 23, 2017

@pmettraux Great, please resolve merge conflicts then I will begin my review 😄

@Playrom
Copy link
Contributor

@Playrom Playrom commented Jul 24, 2017

@pmettraux can you resolve conflicts? or I have to do it? :)

@pmettraux
Copy link
Author

@pmettraux pmettraux commented Jul 27, 2017

I do not have time these days, sorry

@Playrom
Copy link
Contributor

@Playrom Playrom commented Jul 27, 2017

ok I'll do it don't worry :)

@jadengore jadengore removed their request for review Oct 10, 2017
@jadengore jadengore removed their assignment Oct 10, 2017
@ricardograca ricardograca added this to To Do in Version 0.13.0 via automation Jan 9, 2018
@ricardograca ricardograca merged commit e20cbc5 into bookshelf:master Jan 17, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Version 0.13.0 automation moved this from To Do to Done Jan 17, 2018
@ricardograca ricardograca added the bug label Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants