Mongoid 6 conflicts #197

saumyamehta17 · 2017-11-06T16:24:08Z

No description provided.

…d-6-conflicts

dblock · 2017-11-06T16:42:34Z

The check should really run once. Maybe take a dependency on https://github.com/mongoid/mongoid-compatibility and fix using that? We also really need mongoid versions in the test matrix - that repo has a good example on how to do that.

saumyamehta17 · 2017-11-20T01:22:30Z

@dblock added mongoid versions test using mongoid-compatibility. Please Let me know if I missed something.

dblock

I meant to add mongoid-compatibility that adds methods such as mongoid6? and removing any such code here. LMK if I am making no sense :)

dblock · 2017-11-21T19:48:41Z

lib/mongoid/rspec.rb

@@ -38,3 +38,14 @@ module Matchers
    include Mongoid::Matchers::Validations
  end
 end
+
+module Mongoid


This shouldn't exist anymore, it would come from mongoid-compatibility.

dblock · 2017-11-21T19:49:10Z

spec/compatibility/version_spec.rb

@@ -0,0 +1,33 @@
+require 'spec_helper'
+
+RSpec.describe Mongoid::Compatibility::Version do


Why do we need to be testing mongoid-compatibility here?

dblock · 2017-11-21T19:51:22Z

More importantly we/you want to update the .travis.yml configuration here to run against multiple versions of mongoid.

dblock · 2017-12-01T23:47:28Z

Bump @saumyamehta17

saumyamehta17 · 2017-12-04T23:49:30Z

@dblock
Hi Daniel

Sorry for delay in reply, I am bit occupied with my office work. Honestly, I am new to open source contribution and might be asking obvious (dumb) questions/doubts :)

What I understood:

I need to add for support mongoid 6
Also mongoid-compatibility can be added to check multiple versions of mongoid, which adds method to check versions. So, version_spec.rb does not required.
rspec.rb module Mongoid code not needed now.
I need to add all versions of mongoid in travis.yml which will do depenedency tests.

Please let me know if there is any gap.
Thank you for being patient.

dblock · 2017-12-07T15:27:00Z

No worries, I'm here to help!

You got it right. Just keep iterating and I'll try to make things clearer as we make progress. Hang in there.

dblock · 2017-12-19T20:31:32Z

Looks like there're real failures with Mongoid 5, trying to use classes that don't exist in that version.

I think instead of creating separate gemfiles you want something like what we do in https://github.com/mongoid/mongoid-compatibility. Maybe copy from there?

dblock · 2017-12-31T19:35:27Z

lib/matchers/indexes.rb

@@ -0,0 +1,84 @@
+module Mongoid
+  module Matchers
+    class HaveIndexForMongoid3 # :nodoc:


This class should be the same name for different versions. Make lib/matchers/indexes/v3/have_index_for.rb and lib/matchers/indexes/v4/have_index_for.rb for example and include the right file. This will reduce the switching.

dblock · 2017-12-31T19:36:56Z

I've added the Danger token to Travis-CI, so that should be working now.

dblock · 2017-12-31T19:37:36Z

.gitignore

@@ -4,3 +4,47 @@
 Gemfile.lock
 pkg/*

+*.rbc


Don't just copy-paste a bunch of stuff, only things that are needed. See what is being created during build and exclude those.

dblock · 2017-12-31T19:38:22Z

lib/matchers/associations.rb

@@ -1,5 +1,4 @@
 require 'mongoid/relations'
-


Space is cheap, put this back :)

dblock · 2017-12-31T19:39:34Z

lib/matchers/indexes.rb

+        @klass  = klass.is_a?(Class) ? klass : klass.class
+        @errors = []
+
+        if @klass.respond_to?(:index_options)


Since this code is quite different for different versions, break this up into v3/have_index_for.rb, v4/have_index_for.rb, etc. If the index is backward compatible, eg. index for v5 works for v6, just include that one as is.

You might want to inherit common code, so Mongoid::Matchers::HaveIndexFor < Mongoid::Matchers::HaveIndexFor::Base or something like that.

dblock · 2017-12-31T19:40:54Z

lib/matchers/indexes.rb

+    end
+
+    def have_index_for(index_fields)
+      HaveIndexForMongoid3.new(index_fields)


To inherit this you can do self.new in the base class.

dblock · 2017-12-31T19:41:26Z

lib/mongoid/rspec.rb

@@ -38,3 +42,14 @@ module Matchers
    include Mongoid::Matchers::Validations
  end
 end
+
+# module Mongoid


Delete this.

mongoid-bot · 2017-12-31T19:59:53Z

	1 Error
🚫	The CHANGELOG.md file does not exist.

	1 Warning
⚠️	Unless you’re refactoring existing code, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#197](https://github.com/mongoid/mongoid-rspec/pull/197): Mongoid 6 conflicts - [@saumyamehta17](https://github.com/saumyamehta17).

Generated by 🚫 danger

dblock · 2017-12-31T20:06:44Z

Danger is now working, if you want to enable it as part of this PR it needs a CHANGELOG file. Usually I got back a little and try to dig up history from Rubygems, but you can also do this later. Otherwise this build is green, see my comments above. Lets finish it?

dblock

Good progress on subclassing HaveIndexFor, but there's still code that does if in the child class between versions of mongoid that needs to be in version-specific classes. You also have to make Danger happy or just remove it from this PR for now and make that a separate PR in the future (recommended).

dblock · 2018-01-01T18:15:53Z

lib/matchers/indexes/have_index_for.rb

@@ -0,0 +1,18 @@
+module Mongoid
+  module Matchers
+    module Base


Modules are namespaces, so Base isn't a great choice for a module because it doesn't make great sense to organize all base classes together. If all matchers have a base class then just make this class Mongoid::Matchers::Base. If all HaveIndexFor have a base, you can call it Mongoid::Matchers::HaveIndexForBase or even Mongoid::Matchers::HaveIndexFor::Base (just declare HaveIndexFor as a Class for the last case).

dblock · 2018-01-01T18:16:31Z

lib/matchers/indexes/have_index_for.rb

+          @index_options = index_options
+          self
+        end
+


Extra line.

dblock · 2018-01-01T18:18:27Z

lib/matchers/indexes/v3/have_index_for.rb


      def matches?(klass)
        @klass  = klass.is_a?(Class) ? klass : klass.class
        @errors = []

        if @klass.respond_to?(:index_options)
          # Mongoid 3
-          unless @klass.index_options[@index_fields]
-            @errors.push "no index for #{@index_fields}"
+          unless @klass.index_options[@index_key]


I know this code was here before, but it's a good idea to use a property from the base class, so index_key instead of @index_key and klass instead of @klass. Properties should be declared like attr_reader.

dblock · 2018-01-01T18:18:49Z

lib/matchers/indexes/v3/have_index_for.rb

+            if !@index_options.nil? && !@index_options.empty?
+              @index_options.each do |option, option_value|
+                if denormalising_options(@klass.index_options[@index_key])[option] != option_value
+                  @errors.push "index for #{@index_key.inspect} with options of #{@klass.index_options[@index_key].inspect}"
                end
              end
            end
          end
        else
          # Mongoid 4


This should be in a version-specific class.

dblock · 2018-01-01T18:18:58Z

lib/matchers/indexes/v3/have_index_for.rb

-    def have_index_for(index_fields)
-      HaveIndexForMongoid3.new(index_fields)
-    end
+    # Class Ends here


Not a very useful comment ;)

dblock · 2018-01-01T18:20:10Z

lib/mongoid/rspec.rb

@@ -29,10 +29,11 @@
 require 'matchers/be_dynamic_document'
 require 'matchers/be_stored_in'
 require 'matchers/have_field'
+require 'matchers/indexes/have_index_for'
 if Mongoid::Compatibility::Version.mongoid4_or_newer?


Nitpicking, but it's probably better to check whether it's mongoid3? and switch the if around. It's a more exact if.

…ed unwanted lines and comments

dblock · 2018-01-04T19:03:59Z

Merged. Thanks for your excellent work @saumyamehta17 and for hanging in there.

You can/should tackle the following next if you want that weren't blockers:

Compatibility section needs to be updated in README. This gem now works for all the versions out of the box.
There's still code that says this gem is compatible with Mongoid 2 (Gemfile, .gemspec), however we no longer test against mongoid 2. Either put it back and have a passing build or remove it from everywhere.
Links to badges/Travis-CI/codeclimate look wrong in README.
Writing a CHANGELOG and re-adding Danger.
Making the next release after ^.

saumyamehta17 · 2018-01-08T14:46:30Z

Thanks. I will be working. Regards, Saumya Mehta

…

On Fri, Jan 5, 2018 at 12:34 AM, Daniel Doubrovkine (dB.) @dblockdotorg < ***@***.***> wrote: Merged. Thanks for your excellent work @saumyamehta17 <https://github.com/saumyamehta17> and for hanging in there. You can/should tackle the following next if you want that weren't blockers: - Compatibility section needs to be updated in README. This gem now works for all the versions out of the box. - There's still code that says this gem is compatible with Mongoid 2 (Gemfile, .gemspec), however we no longer test against mongoid 2. Either put it back and have a passing build or remove it from everywhere. - Links to badges/Travis-CI/codeclimate look wrong in README. - Writing a CHANGELOG and re-adding Danger. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#197 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD169LK2RW_iZudUcfhgrRDjmk62CjUTks5tHSCggaJpZM4QTf7e> .

khaight · 2018-01-08T18:50:28Z

Nice fix. FYI - I noticed that I needed to add the mongoid-compatibility gem to my Gemfile with this change.

  gem 'mongoid-compatibility', '~> 0.5.1'
  gem 'mongoid-rspec', github: 'mongoid-rspec/mongoid-rspec'

I see that it is included in the .gemspec file as a dependency, but my rails project did not pick it up. Maybe has something to do with including gem 'mongoid-rspec', github: 'mongoid-rspec/mongoid-rspec' in my group :test do block?

dblock · 2018-01-09T23:32:05Z

@khaight What happens without? You should raise a bug. We include that dependency in https://github.com/mongoid/mongoid-rspec/blob/master/mongoid-rspec.gemspec#L24 and it shouldn't be required.

aptx4869 · 2018-01-14T02:43:46Z

@dblock without adding mongoid-compatibility gem to Gemfile before mongoid-rspec will raise an uninitialized constant Mongoid::Compatibility error.

~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:84:in `rescue in block (2 levels) in require': There was an error
while trying to load the gem 'mongoid-rspec'.
Gem Load Error is: uninitialized constant Mongoid::Compatibility
Backtrace for gem load error is:
~/.rvm/gems/ruby-2.4.1/bundler/gems/mongoid-rspec-472f1460c23d/lib/mongoid/rspec.rb:8:in `<top (required)>'
~/.rvm/gems/ruby-2.4.1/bundler/gems/mongoid-rspec-472f1460c23d/lib/mongoid-rspec.rb:1:in `<top (required)>'
~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:81:in `require'
~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:81:in `block (2 levels) in require'
~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:76:in `each'
~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:76:in `block in require'
~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:65:in `each'
~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:65:in `require'
~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler.rb:114:in `require'
./config/application.rb:17:in `<top (required)>'
~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:92:in `require'
~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:92:in `preload'
~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:153:in `serve'
~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:141:in `block in run'
~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:135:in `loop'
~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:135:in `run'
~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application/boot.rb:19:in `<top (required)>'
~/.rvm/rubies/ruby-2.4.1/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'
~/.rvm/rubies/ruby-2.4.1/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'
-e:1:in `<main>'
Bundler Error Backtrace:
 (Bundler::GemRequireError)
        from ~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:80:in `block (2 levels) in require'
        from ~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:76:in `each'
        from ~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:76:in `block in require'
        from ~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:65:in `each'
        from ~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler/runtime.rb:65:in `require'
        from ~/.rvm/gems/ruby-2.4.1/gems/bundler-1.16.1/lib/bundler.rb:114:in `require'
        from ./config/application.rb:17:in `<top (required)>'
        from ~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:92:in `require'
        from ~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:92:in `preload'
        from ~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:153:in `serve'
        from ~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:141:in `block in run'
        from ~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:135:in `loop'
        from ~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application.rb:135:in `run'
        from ~/.rvm/gems/ruby-2.4.1/gems/spring-2.0.2/lib/spring/application/boot.rb:19:in `<top (required)>'
        from ~/.rvm/rubies/ruby-2.4.1/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from ~/.rvm/rubies/ruby-2.4.1/lib/ruby/site_ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'
        from -e:1:in `<main>'

dblock · 2018-01-14T16:49:06Z

Looks legit. I opened #203. Would love some help with a fix @aptx4869. Also there're a few other open issues before we can release v. next like #201 #202, etc., any help appreciated.

zeitnot and others added 3 commits September 24, 2016 00:41

Support released mongoid 6.0.0

0a8159b

change the dependency criteria of mongoid.

328d255

Merge branch 'master' of github.com:mongoid/mongoid-rspec into mongoi…

c8e349d

…d-6-conflicts

saumyamehta17 added 2 commits November 20, 2017 06:34

mongoid versions added in test matrix

7a22a6e

removed extra space

a432712

dblock requested changes Nov 21, 2017

View reviewed changes

This was referenced Dec 1, 2017

Mongoid 6.x support? #198

Closed

Mongoid 6 support. #195

Closed

saumyamehta17 added 3 commits December 16, 2017 06:42

travis changes

51834c1

gemspec path corrected

dc99a60

typo

ab907d3

saumyamehta17 added 2 commits December 30, 2017 20:20

multi mongoid version support added

be436a6

versions gemfiles removed

939babe

dblock reviewed Dec 31, 2017

View reviewed changes

lib/matchers/associations.rb Outdated

@@ -1,5 +1,4 @@

require 'mongoid/relations'

Copy link

Collaborator

dblock Dec 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space is cheap, put this back :)

dblock reviewed Dec 31, 2017

View reviewed changes

reviews comments incorporated

8b9d01e

dblock requested changes Jan 1, 2018

View reviewed changes

have_index_for correction per versions, removed danger for now, remov…

4debd16

…ed unwanted lines and comments

dblock merged commit a2db3ec into mongoid:master Jan 4, 2018

This was referenced Jan 4, 2018

Needs more maintainers #196

Closed

Release v. Next #193

Closed

Add Danger CI and CHANGELOG #199

Closed

Fix README compatibility and badges #201

Closed

dblock mentioned this pull request Jan 14, 2018

uninitialized constant Mongoid::Compatibility #203

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mongoid 6 conflicts #197

Mongoid 6 conflicts #197

saumyamehta17 commented Nov 6, 2017

dblock commented Nov 6, 2017

saumyamehta17 commented Nov 20, 2017

dblock left a comment

dblock Nov 21, 2017

dblock Nov 21, 2017

dblock commented Nov 21, 2017 •

edited

Loading

dblock commented Dec 1, 2017

saumyamehta17 commented Dec 4, 2017

dblock commented Dec 7, 2017

dblock commented Dec 19, 2017

dblock Dec 31, 2017

dblock commented Dec 31, 2017

dblock Dec 31, 2017

dblock Dec 31, 2017

dblock Dec 31, 2017

dblock Dec 31, 2017

dblock Dec 31, 2017

dblock Dec 31, 2017

mongoid-bot commented Dec 31, 2017 •

edited

Loading

dblock commented Dec 31, 2017

dblock left a comment

dblock Jan 1, 2018 •

edited

Loading

dblock Jan 1, 2018

dblock Jan 1, 2018

dblock Jan 1, 2018

dblock Jan 1, 2018

dblock Jan 1, 2018

dblock commented Jan 4, 2018 •

edited

Loading

saumyamehta17 commented Jan 8, 2018 via email

khaight commented Jan 8, 2018 •

edited

Loading

dblock commented Jan 9, 2018

aptx4869 commented Jan 14, 2018

dblock commented Jan 14, 2018

		@@ -0,0 +1,33 @@
		require 'spec_helper'

		RSpec.describe Mongoid::Compatibility::Version do

Mongoid 6 conflicts #197

Mongoid 6 conflicts #197

Conversation

saumyamehta17 commented Nov 6, 2017

dblock commented Nov 6, 2017

saumyamehta17 commented Nov 20, 2017

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dblock commented Nov 21, 2017 • edited Loading

dblock commented Dec 1, 2017

saumyamehta17 commented Dec 4, 2017

dblock commented Dec 7, 2017

dblock commented Dec 19, 2017

Choose a reason for hiding this comment

dblock commented Dec 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mongoid-bot commented Dec 31, 2017 • edited Loading

dblock commented Dec 31, 2017

dblock left a comment

Choose a reason for hiding this comment

dblock Jan 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dblock commented Jan 4, 2018 • edited Loading

saumyamehta17 commented Jan 8, 2018 via email

khaight commented Jan 8, 2018 • edited Loading

dblock commented Jan 9, 2018

aptx4869 commented Jan 14, 2018

dblock commented Jan 14, 2018

dblock commented Nov 21, 2017 •

edited

Loading

mongoid-bot commented Dec 31, 2017 •

edited

Loading

dblock Jan 1, 2018 •

edited

Loading

dblock commented Jan 4, 2018 •

edited

Loading

khaight commented Jan 8, 2018 •

edited

Loading