Switch .uniq to .distinct while dropping rails 3.2 Support

[rposborne]

See #52

[dwbutler]

I resolved some dependency issues that were causing CI to fail. Please rebase from master.

I was planning to hold off on this until Rails 5.1. was released. I would prefer not to drop Rails 3.2 support until absolutely necessary.

Is there a motivating factor for this, beyond getting rid of the annoying deprecation warnings?

[rposborne]

Rebased

@dwbutler No other motivation other than the warning / Trying to help out, assumed that master would track the upcoming release.

This PR can sit in purgatory for the time being if you don't want to merge yet.

[dwbutler]

Sounds good. Thanks for helping out! I'll leave this open until Rails 5.1 is released, and then I'll merge it and drop Rails 3.2 support officially.

Also, the changelog is auto-generated, so you can remove the change there.

[dwbutler]

Also, Rails 3.2 needs to be removed from the Travis CI build.

[rposborne]

Any thoughts on removing End of Life (EOL) versions of ruby from the build matrix? It appears we are building over 100 times per commit.

[dwbutler]

I usually continue supporting older Rubies until it becomes too painful. (Usually because of test dependencies.)

AFAIK, Ruby 2.0.0 is the only version that has been officially EOLed. I would be fine with removing it.

[dwbutler]

The conditional here can be removed since Rails 3 will no longer be supported or expected to work.

[dwbutler]

@rposborne Rails 5.1 is out. We can also remove support for Ruby 2.1, since it was recently EOL'ed.

[rposborne]

3 Failures for 5.1, traced it to here. rails/rails@09cac8c

But I seem to sense that we weren't hitting this before, and some code is not being loaded when it needs to be.

[dwbutler]

@rposborne The failing specs are here and here. To be honest, this looks like invalid syntax, and I'm not sure how these tests ever passed. In any case, it's not syntax that should be supported. So these lines can be removed.

[rposborne]

@dwbutler I tend to agree, but it appears this syntax is working from other perspectives... which has me confused as I am not sure where the code path for the successful call is coming from.

Here is a working example of the same lookup.
https://github.com/dwbutler/groupify/blob/master/spec/active_record_spec.rb#L394

[dwbutler]

@rposborne The syntax user.groups(as: 'role') doesn't actually work. This is equivalent to writing user.groups(true) which forces the association to reload. The tests aren't written in such a way that they would correctly fail with this incorrect usage.

[dwbutler]

@rposborne Actually, I found the code that implements this behavior here.

Evidently it doesn't work any more in ActiveRecord 5.1. I would suggest that it be removed. The behavior isn't documented anywhere.

[dwbutler]

@rposborne For now you can add this to the allowed_failures in .travis.yml:

    - rvm: jruby-9.1.5.0
      gemfile: gemfiles/rails_5.1.gemfile

[rposborne]

@dwbutler Should be in good shape! Final review :) I do think this is a major version bump regardless with the syntax deprecation and dropping of support of rubies / rails.

[dwbutler]

Awesome, thanks for all the hard work!

I don't think a major bump is necessarily needed here. The API documented in the Readme is the only officially supported interface. The internals can change/break as needed. But definitely this will bump the version from 0.8.0 to 0.9.0.

[dwbutler]

Released in 0.9.0. Thanks!