Skip to content

Commit

Permalink
Call context-specific includes for each taggable_on call
Browse files Browse the repository at this point in the history
Don't include Utils multiple times
  • Loading branch information
shekibobo committed Apr 13, 2013
1 parent c40ecc4 commit e44b69d
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions lib/acts_as_taggable_on/taggable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,17 @@ def self.taggable?
end

include ActsAsTaggableOn::Utils
include ActsAsTaggableOn::Taggable::Core
include ActsAsTaggableOn::Taggable::Collection
include ActsAsTaggableOn::Taggable::Cache
include ActsAsTaggableOn::Taggable::Ownership
include ActsAsTaggableOn::Taggable::Related
include ActsAsTaggableOn::Taggable::Dirty
end
end

# each of these add context-specific methods and must be
# called on each call of taggable_on
include ActsAsTaggableOn::Taggable::Core
include ActsAsTaggableOn::Taggable::Collection
include ActsAsTaggableOn::Taggable::Cache
include ActsAsTaggableOn::Taggable::Ownership
include ActsAsTaggableOn::Taggable::Related
include ActsAsTaggableOn::Taggable::Dirty
end

end
Expand Down

3 comments on commit e44b69d

@ches
Copy link
Contributor

@ches ches commented on e44b69d Sep 26, 2013

Choose a reason for hiding this comment

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

There are no tests and no issue referenced -- is there a demonstration somewhere of what this was supposed to fix? This change undid the work of 16c4567 -- it now re-includes most of the library every time subsequent calls are made like:

acts_as_taggable_on :pros
acts_as_taggable_on :cons

All of the modules define hooks like this so that the full includes shouldn't be needed every time:

def acts_as_taggable_on(*args)
  super(*args)
  initialize_acts_as_taggable_on_collection
end

Those initialize_* class methods should do the work of dynamically defining new context-specific methods on subsequent calls of the macro.

@ches
Copy link
Contributor

@ches ches commented on e44b69d Sep 26, 2013

Choose a reason for hiding this comment

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

Sorry, it's #339 -- having a look over it now.

@ches
Copy link
Contributor

@ches ches commented on e44b69d Sep 26, 2013

Choose a reason for hiding this comment

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

Addressed in #411.

Please sign in to comment.