search

mislav / will_paginate

Pagination library for Rails and other Ruby applications
http://github.com/mislav/will_paginate/wikis
MIT License
5.71k stars 864 forks source link

Rails 6.0 & 6.1 support #619

Closed kvokka closed 3 years ago

kvokka commented 3 years ago

While rails < 6 used 2 arguments for defaults block, rails 6 use only 1 argument.

This patch makes the code runnable with both options

Connect replaygaming/replaypoker#4860 Close mislav/will_paginate#618

alejandrodevs commented 3 years ago

Any progress on this PR?

kvokka commented 3 years ago

@hamityay can u please give me another look at this?

By the way, i fixed broken CI (for the latest rails) and added separate gemfiles for rail 6 & 6.1

alejandrodevs commented 3 years ago

@kvokka travis is still failing :(

kvokka commented 3 years ago

@alejandrodevs not now ;)

kvokka commented 3 years ago

@hamityay just noticed that i'm not able to re-request the review, so doing it directly

hamityay commented 3 years ago

@kvokka looks good to me, but not sure I am the right person to approve changes to get in

mislav commented 3 years ago

Thank you for the proposed fix! I was confused about this specific fix because:

  1. It would hint that the underlying I18n library somehow changed how it handles Procs, whereas it hasn't;
  2. Your added tests pass on Rails 6.1 even without your fix applied.

I went looking into the Rails 6.1 breakage and concluded that the problem isn't in will_paginate, but in Rails itself. See more info in the original issue.