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.

Sign up for GitHub

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

Reduced method's name length #442

Merged
merged 1 commit into from
Dec 27, 2013
Merged

Reduced method's name length #442

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions lib/acts_as_taggable_on/acts_as_taggable_on/cache.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@ def self.included(base)
# has any "cached_#{tag_type}_list" column
base.instance_eval do
# @private
def _has_acts_as_taggable_on_cache_columns?(db_columns)
def _has_tags_cache_columns?(db_columns)
db_column_names = db_columns.map(&:name)
tag_types.any? {|context|
db_column_names.include?("cached_#{context.to_s.singularize}_list")
}
end

# @private
def _add_acts_as_taggable_on_caching_methods
def _add_tags_caching_methods
send :include, ActsAsTaggableOn::Taggable::Cache::InstanceMethods
extend ActsAsTaggableOn::Taggable::Cache::ClassMethods

before_save :save_cached_tag_list

initialize_acts_as_taggable_on_cache
initialize_tags_cache
end

# ActiveRecord::Base.columns makes a database connection and caches the calculated
Expand All @@ -32,8 +32,8 @@ def _add_acts_as_taggable_on_caching_methods
def columns
@acts_as_taggable_on_columns ||= begin
db_columns = super
if _has_acts_as_taggable_on_cache_columns?(db_columns)
_add_acts_as_taggable_on_caching_methods
if _has_tags_cache_columns?(db_columns)
_add_tags_caching_methods
end
db_columns
end
Expand All @@ -43,7 +43,7 @@ def columns
end

module ClassMethods
def initialize_acts_as_taggable_on_cache
def initialize_tags_cache
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess that method was never explicitly tested. (Its name is the the convention I was following in naming the methods I added)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think there's any chance anyone might be calling that manually?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As part of a test (rspec, mini-test) maybe but unlikely. i don't think anyone will need to call it from within the controllers/helpers

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, based on working with you over these last few issues I would recommend you as additional maintainer of the project, if you're interested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With great pleasure. Thank you

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey, @tilsammans @mbleigh @artemk I'm recommending @seuros as another co-maintainer. (And plz help with #441 if you can!)

tag_types.map(&:to_s).each do |tag_type|
class_eval <<-RUBY, __FILE__, __LINE__ + 1
def self.caching_#{tag_type.singularize}_list?
Expand All @@ -55,7 +55,7 @@ def self.caching_#{tag_type.singularize}_list?

def acts_as_taggable_on(*args)
super(*args)
initialize_acts_as_taggable_on_cache
initialize_tags_cache
end

def caching_tag_list_on?(context)
Expand Down