-
Notifications
You must be signed in to change notification settings - Fork 24
incremented versions for gems to rails 6 ruby 2.7.0 and mongo 7 #6
base: master
Are you sure you want to change the base?
incremented versions for gems to rails 6 ruby 2.7.0 and mongo 7 #6
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.
This PR looks good in general, but I commented some stuff. Thanks!
@@ -1,5 +1,5 @@ | |||
module Ransack | |||
module Mongoid | |||
VERSION = "0.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.
I don't think you need to bump the version, since nothing is really changing from a final user perspective. This PR is just making sure that this gem works against newest versions of dependencies.
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.
Without the version bump, the gem does not update and it does not resolve the new decency versions.
spec.version = Ransack::Mongoid::VERSION | ||
spec.authors = ["Ernie Miller", "Ryan Bigg", "Jon Atack","Sean Carroll"] |
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 don't think you get to be added as the author of this gem just with a single contribution 😅. I'm grateful, and probably the authors of this gem too, for your contribution though.
In fact, I would personally revert all the changes in this file, since you only changing the quoting style, and adding your name and email address.
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 for reviewing so intensely. Is there anything that is missing or that requires attention for the gem to be ready for 6 ruby 2.7.0 and mongo 7 ?
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 have no idea, since it doesn't even seem this repo has TravisCI enabled. We can probably ask the current maintainers to add that?
Have you tried the gem in your application?
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 am currently using this version, hence the PR request.
else | ||
gem 'mongoid', '~> 5.0.0', require: false |
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.
This gem should probably declare an official range of supported mongoid versions, and explicitly test those in CI. That sounds like something for a separate PR, though.
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 tried that but it breaks the older version.
could some of the maintainers take care of it? we want to add this functionality to activeadmin-mongoid. |
@grzegorz-jakubiak there are no maintainers, have you considered becoming one? |
|
M .travis.yml
M Gemfile
M Gemfile.lock
M ransack-mongoid.gemspec