Add Ruby 2.3, 2.4, 2.5, and 2.6 and Mongoid 6 and 7 #20

yuki24 · 2019-05-14T04:02:51Z

It seems like this repo is a bit behind the pace and I thought we should test against more recent versions of Ruby and Mongoid. I got most of the new builds green except for Mongoid 7.

yuki24 · 2019-05-14T04:26:27Z

I had to drop builds for Ruby 1.9.3. It doesn't seem to install on Travis any more and it's very old, so I don't think there's a good reason to maintain support for that.

This fixes the error below: An error occurred while loading ./spec/array_spec.rb. Failure/Error: include ActiveSupport::Callbacks NoMethodError: undefined method `class_attribute' for Mongoid::CachedJson::Config:Module

dblock · 2019-05-14T14:06:08Z

spec/support/json_manager.rb

@@ -5,7 +5,12 @@ class JsonManager
  field :name
  field :ssn, default: '123-45-6789'
  has_many :json_employees
-  belongs_to :supervisor, class_name: 'JsonSupervisor'
+
+  if Mongoid::VERSION < "6.0.0"


This is not a valid comparison! Use https://github.com/mongoid/mongoid-compatibility 's Mongoid::Compatibility::Version.mongoid6_or_older?

It is totally a valid comparison that people think is silly that works totally fine:

"6.0.1" < "6.0.0" # => false "6.0.0" < "6.0.0" # => false "5.10.10" < "6.0.0" # => true "5.9.9" < "6.0.0" # => true

I don't think we need a dependency.

Let's take a dependency?

"6.0.0" > "6.0" # => true

Also note that the dependency would only be for tests.

Another example that fails.

"6.0.0" < "6.0.0.beta" # => true

I'd use Gem::Version if a more accurate comparison is necessary:

Gem::Version.new("6.0.0") < Gem::Version.new("6.0") # => false Gem::Version.new("6.0.1") < Gem::Version.new("6.0.0") # => false Gem::Version.new("6.0.0") < Gem::Version.new("6.0.0") # => false Gem::Version.new("5.10.10") < Gem::Version.new("6.0.0") # => true Gem::Version.new("5.9.9") < Gem::Version.new("6.0.0") # => true

That's what mongoid-compatibility does, but smarter and future-proof as it checks major version only and has semantic verbs like mongoid6_or_older? which are easier to read. It just has been convention in mongoid projects since we were doing these comparisons in 10 different ways all over the place.

https://github.com/mongoid/mongoid-compatibility/blob/master/lib/mongoid/compatibility/version.rb

as it checks major version only and has semantic verbs like mongoid6_or_older? which are easier to read.

This doesn't sound like a strong counter objection when the Gem::Version comparizon is very simple and easy to understand. Is this something that might take more than an hour to understand, or something that really requires encapsulation?

since we were doing these comparisons in 10 different ways all over the place.

Here we are not trying to understand 10 different sorting algorithms, but just a version comparison, basically a number comparison internally in Gem::Version. Also, could you actually really list 10 different examples? I have seen a few, but I'm sure it doesn't exceed 10.

I noticed it's already a dependency of this project, so I just switched it anyway. However, I'm not convinced that this is a better way as Gem::Version is a more generic abstraction that can be used almost anywhere else, and it doesn't require any specific knowledge about Mongoid.

I think it's all a small deal and either works!

dblock · 2019-05-14T14:06:57Z

Rakefile

@@ -1,7 +1,9 @@
+# frozen_string_literal: true


I think these should go away, we either do it everywhere or nowhere.

Rubocop disagrees. What would you prefer?

Actually it stopped complaining about it after running with a --auto-gen-config config, so this should be good now.

I'm ok with it.

Personally, I like these to be consistent. So I generally don't have the #frozen in there and disable it in Rubocop or --auto-gen-config. This was added in this diff, that's why I am calling it out. You could revert this file and re-run --auto-gen-config and it would be better.

dblock · 2019-05-14T14:08:21Z

Rakefile

@@ -31,4 +33,8 @@ end
 require 'rubocop/rake_task'
 RuboCop::RakeTask.new

-task default: [:rubocop, :spec]
+if RUBY_VERSION >= '2.6'


Why? Just to save Running rubocop? This feels like it assumes content in .travis.yml and will be hard to debug. Furthermore anyone contributing on Ruby 2.5.3 will forget that Rubocop runs until they push and get a failure. How about changing the rake parameters in .travis.yml and running rubocop for only one matrix line in there?

Alright, I was able to add a separate build that only runs rake rubocop and the default rake now only runs the spec task.

dblock · 2019-05-14T14:09:16Z

CHANGELOG.md

 ------------

 * Your contribution here.
+* Compatibility with Ruby 2.3, 2.4, 2.5 and 2.6 - [@yuki24](http://github.com/yuki24).


Care to prefix these with a PR link?

[#20](https://github.com/mongoid/mongoid-cached-json/pull/20): Compatibility ...

dblock

Thanks!

The README contains a Compatibility section that also needs to be updated. Also since it no longer works, please get rid of the gemnasium badge while you're there.

dblock · 2019-05-14T16:36:42Z

@yuki24 I also invited you to collaborators on this repo cause I know you're dying to co-maintain this and make the next release :) Drop me your rubygems email to dblock at dblock dot org?

dblock · 2019-05-14T16:37:36Z

spec/cached_json_spec.rb

-          peon = manager.json_employees.create(name: 'Peon')
-          manager.json_employees.create(name: 'Indentured servant')
-          manager.json_employees.create(name: 'Serf', nickname: 'Vince')
+          manager = JsonManager.create!(name: 'Boss')


dblock · 2019-05-14T16:42:17Z

CHANGELOG.md

@@ -1,7 +1,10 @@
-1.5.4 (Next)
+2.0.0 (Next)


Bump version.rb or undo.

Since the next version is actually compatible with newer mongoid's without changes, and the only "breaking" change is that we drop testing or support on 1.9.3, I would just leave it as 1.5.4, but up to you.

Right, the next version will probably be compatible with 1.9.3. What version would you prefer? 1.6.0 or you still prefer 1.5.4?

I've picked up 1.6.0 to follow semver, but let me know if you have any other preferences.

That works!

yuki24 · 2019-05-14T16:57:52Z

.travis.yml

+    - rvm: 2.1.10
+      env: MONGOID_VERSION=7
+  allow_failures:
+    - env: MONGOID_VERSION=7


Just to be clear, I couldn't get the builds for Mongoid 7 so leaving this here for now, but I would like to get them pass once I have enough time to fix them.

Ok let's deal with that next.

yuki24 · 2019-05-14T17:07:13Z

Rakefile

@@ -31,4 +31,4 @@ end
 require 'rubocop/rake_task'
 RuboCop::RakeTask.new

-task default: [:rubocop, :spec]
+task default: :spec


@dblock are you okay with this? I think this might be suprizing for people who use Rubocop regularly.

I would leave the default as is and run both. This is just because it has been this way for a long time and we like having rubocop in the mongoid org.

dblock · 2019-05-14T18:57:46Z

I merged this as is. Let's deal with the other issues next.

dblock · 2019-05-14T18:57:56Z

Thanks for your good work @yuki24 !

dblock · 2019-05-14T19:00:54Z

For Mongoid 7, the changes should be minimal with Mongoid::Relations becoming associations. See https://github.com/mongoid/mongoid-history/pull/221/files for some examples.

yuki24 added 2 commits May 14, 2019 01:20

Test against Ruby 2.1/2.3/2.4/2.5/2.6 and Mongoid 6/7

feebb0f

Fixes an NoMethodError on loading config.r

169c344

This fixes the error below: An error occurred while loading ./spec/array_spec.rb. Failure/Error: include ActiveSupport::Callbacks NoMethodError: undefined method `class_attribute' for Mongoid::CachedJson::Config:Module

dblock reviewed May 14, 2019

View reviewed changes

dblock requested changes May 14, 2019

View reviewed changes

dblock reviewed May 14, 2019

View reviewed changes

yuki24 commented May 14, 2019

View reviewed changes

yuki24 added 6 commits May 14, 2019 13:39

Fix builds for Mongoid 6

d482d86

Relax rubocop version constraints

e56e769

Do not test against Ruby 1.9.3 any more

7c6d249

No need to specify the version of bundler in Gemfile

ea33a8f

Run a separate build for Rubocop

83dd3bb

Update CHANGELOG and README

ad17c47

dblock merged commit 0c9c6a6 into mongoid:master May 14, 2019

yuki24 deleted the test-against-more-mongoids branch May 18, 2019 01:12

yuki24 mentioned this pull request May 31, 2019

Preparing for release, 1.6.0 #24

Merged

Add Ruby 2.3, 2.4, 2.5, and 2.6 and Mongoid 6 and 7 #20

Add Ruby 2.3, 2.4, 2.5, and 2.6 and Mongoid 6 and 7 #20

Conversation

yuki24 commented May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuki24 commented May 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuki24 May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblock May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblock left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblock commented May 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblock commented May 14, 2019

Uh oh!

dblock commented May 14, 2019

Uh oh!

dblock commented May 14, 2019

Uh oh!

yuki24 commented May 14, 2019 •

edited

Loading

yuki24 May 14, 2019 •

edited

Loading

dblock May 14, 2019 •

edited

Loading

dblock left a comment •

edited

Loading