Skip to content
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

Fix deprecation in has_many associations #32

Merged
merged 1 commit into from
May 27, 2017
Merged

Fix deprecation in has_many associations #32

merged 1 commit into from
May 27, 2017

Conversation

jonian
Copy link
Contributor

@jonian jonian commented May 24, 2017

DEPRECATION WARNING: Passing a class to the class_name is deprecated and will raise an ArgumentError in Rails 5.2. It eagerloads more classes than necessary and potentially creates circular dependencies. Please pass the class name as a string: has_many :mobility_text_translations, class_name: 'Mobility::ActiveRecord::TextTranslation'

@shioyama
Copy link
Owner

Thanks very much for this! I noticed some specs are failing not related to your changes here. Once #34 passes I'll merge that and you can rebase, and specs should pass.

@shioyama
Copy link
Owner

Ok #34 has been merged, can you rebase this? 😄

@@ -78,7 +78,7 @@ def self.configure(options)

has_many association_name, ->{ where key: association_attributes },
as: :translatable,
class_name: translations_class,
class_name: "#{translations_class}",
Copy link
Owner

Choose a reason for hiding this comment

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

Could this just be translations_class.to_s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you prefer it that way I will change it.

 Passing a class to the `class_name` is deprecated and will raise an ArgumentError in Rails 5.2
@shioyama
Copy link
Owner

Ok I've started running tests against Rails 5.1 in the active_record_5.x branch, and as expected there are a few issues, many deprecation warnings and some failing specs. I'd like to handle these together, let me have a look and get back to this.

@shioyama shioyama merged commit fcdf5aa into shioyama:master May 27, 2017
@shioyama
Copy link
Owner

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants