-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
update kaminari dependency to 1.x #1734
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be less intrusive
core/solidus_core.gemspec
Outdated
@@ -28,7 +28,7 @@ Gem::Specification.new do |s| | |||
s.add_dependency 'ffaker', '~> 2.0' | |||
s.add_dependency 'friendly_id', '~> 5.0' | |||
s.add_dependency 'highline', '~> 1.7' # Necessary for the install generator | |||
s.add_dependency 'kaminari', '~> 0.15', '>= 0.15.1' | |||
s.add_dependency 'kaminari', '~> 1.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are forcing stores to use Kaminari 1.0 then (which has some breaking changes), while ['>= 0.17', '< 2.0']
would be less intrusive and still Rails 5 compatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that makes sense, I'll change it to be that. (I'm not sure it actually has any intentional breaking changes, I couldn't find any; but may have unintentional breaking changes and bugs; and I agree with the principle to not force people to upgrade)
This was allowing as low as 0.15.1
before, does that not work on Rails5 at all, I should require at least
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/kaminari/kaminari/blob/master/CHANGELOG.md#breaking-changes
The most concerning one is the removal of num_pages
on AR scopes.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
2dcafa5
to
f99282c
Compare
f99282c
to
99fa290
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 🍬
https://github.com/kaminari/kaminari/blob/master/CHANGELOG.md