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 dependency issue with how TokenMaster::Model module is added to A… #48

Merged
merged 1 commit into from
Oct 30, 2017

Conversation

francirp
Copy link
Contributor

Resolves #47

Since we're using the on_load hook, self is actually the ApplicationRecord class (or ActiveRecord::Base for Rails 4), so there is no need to use the class name when including our module. Referencing ApplicationRecord was creating a circular dependency, though I'm not sure why.

Here is an article that explains it more: https://simonecarletti.com/blog/2011/04/understanding-ruby-and-rails-lazy-load-hooks/

Copy link
Contributor

@inveterateliterate inveterateliterate left a comment

Choose a reason for hiding this comment

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

That is really interesting that we didn't get an error earlier. I can't remember why we included it like this to begin with, but good find!

Other notes:

https://github.com/rails/rails/blob/v3.0.6/activesupport/lib/active_support/lazy_load_hooks.rb (I see then that since we're passing active_record to the block, it is the self)

https://blog.bigbinary.com/2015/12/28/application-record-in-rails-5.html (we didn't really need to separate rails 5 from non-rails 5 to begin with either)

@francirp francirp merged commit ff8c32a into master Oct 30, 2017
@inveterateliterate inveterateliterate deleted the active-record-autoloading-issue branch November 7, 2017 16:20
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