Skip to content

Commit

Permalink
Removed ruby deprecated syntax
Browse files Browse the repository at this point in the history
  • Loading branch information
seuros committed Apr 18, 2014
1 parent cfc85bc commit 4c5314b
Show file tree
Hide file tree
Showing 44 changed files with 469 additions and 489 deletions.
4 changes: 2 additions & 2 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ require 'rubygems'
require 'bundler/setup'

desc 'Default: run specs'
task :default => :spec
task default: :spec

desc 'Copy sample spec database.yml over if not exists'
task :copy_db_config do
cp 'spec/internal/config/database.yml.sample', 'spec/internal/config/database.yml'
end

task :spec => [:copy_db_config]
task spec: [:copy_db_config]

require 'rspec/core/rake_task'
RSpec::Core::RakeTask.new do |t|
Expand Down
6 changes: 3 additions & 3 deletions db/migrate/1_acts_as_taggable_on_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ def self.up

# You should make sure that the column created is
# long enough to store the required class names.
t.references :taggable, :polymorphic => true
t.references :tagger, :polymorphic => true
t.references :taggable, polymorphic: true
t.references :tagger, polymorphic: true

# Limit is created to prevent MySQL error on index
# length for MyISAM table type: http://bit.ly/vgW2Ql
t.string :context, :limit => 128
t.string :context, limit: 128

t.datetime :created_at
end
Expand Down
8 changes: 3 additions & 5 deletions db/migrate/2_add_missing_unique_indices.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
class AddMissingUniqueIndices < ActiveRecord::Migration

def self.up
add_index :tags, :name, unique: true

remove_index :taggings, :tag_id
remove_index :taggings, [:taggable_id, :taggable_type, :context]
add_index :taggings,
[:tag_id, :taggable_id, :taggable_type, :context, :tagger_id, :tagger_type],
unique: true, name: 'taggings_idx'
end
[:tag_id, :taggable_id, :taggable_type, :context, :tagger_id, :tagger_type],
unique: true, name: 'taggings_idx'
end

def self.down
remove_index :tags, :name
Expand All @@ -17,5 +16,4 @@ def self.down
add_index :taggings, :tag_id
add_index :taggings, [:taggable_id, :taggable_type, :context]
end

end
2 changes: 1 addition & 1 deletion db/migrate/3_add_taggings_counter_cache_to_tags.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class AddTaggingsCounterCacheToTags < ActiveRecord::Migration
def self.up
add_column :tags, :taggings_count, :integer, :default => 0
add_column :tags, :taggings_count, :integer, default: 0

ActsAsTaggableOn::Tag.reset_column_information
ActsAsTaggableOn::Tag.find_each do |tag|
Expand Down
3 changes: 1 addition & 2 deletions lib/acts-as-taggable-on.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def self.respond_to?(method_name, include_private=false)
def self.glue
setting = @configuration.delimiter
delimiter = setting.kind_of?(Array) ? setting[0] : setting
delimiter.ends_with?(" ") ? delimiter : "#{delimiter} "
delimiter.ends_with?(' ') ? delimiter : "#{delimiter} "
end

class Configuration
Expand All @@ -45,7 +45,6 @@ def initialize
setup
end


require 'acts_as_taggable_on/utils'

require 'acts_as_taggable_on/taggable'
Expand Down
5 changes: 1 addition & 4 deletions lib/acts_as_taggable_on/acts_as_taggable_on/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ def _add_tags_caching_methods
def columns
@acts_as_taggable_on_cache_columns ||= begin
db_columns = super
if _has_tags_cache_columns?(db_columns)
_add_tags_caching_methods
end
_add_tags_caching_methods if _has_tags_cache_columns?(db_columns)
db_columns
end
end
Expand Down Expand Up @@ -79,6 +77,5 @@ def save_cached_tag_list
true
end
end

end
end
14 changes: 6 additions & 8 deletions lib/acts_as_taggable_on/acts_as_taggable_on/collection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ def #{tag_type.singularize}_counts(options = {})
end
def top_#{tag_type}(limit = 10)
tag_counts_on('#{tag_type}', :order => 'count desc', :limit => limit.to_i)
tag_counts_on('#{tag_type}', order: 'count desc', limit: limit.to_i)
end
def self.top_#{tag_type}(limit = 10)
tag_counts_on('#{tag_type}', :order => 'count desc', :limit => limit.to_i)
tag_counts_on('#{tag_type}', order: 'count desc', limit: limit.to_i)
end
RUBY
end
Expand All @@ -34,11 +34,11 @@ def acts_as_taggable_on(*args)
end

def tag_counts_on(context, options = {})
all_tag_counts(options.merge({:on => context.to_s}))
all_tag_counts(options.merge({on: context.to_s}))
end

def tags_on(context, options = {})
all_tags(options.merge({:on => context.to_s}))
all_tags(options.merge({on: context.to_s}))
end

##
Expand Down Expand Up @@ -75,7 +75,6 @@ def all_tags(options = {})
tag_scope_joins(tag_scope, tagging_scope)
end


##
# Calculate the tag counts for all tags.
#
Expand All @@ -98,7 +97,6 @@ def all_tag_counts(options = {})
taggable_join = "INNER JOIN #{table_name} ON #{table_name}.#{primary_key} = #{ActsAsTaggableOn::Tagging.table_name}.taggable_id"
taggable_join << " AND #{table_name}.#{inheritance_column} = '#{name}'" unless descends_from_active_record? # Current model is STI descendant, so add type checking to the join condition


## Generate scope:
tagging_scope = ActsAsTaggableOn::Tagging.select("#{ActsAsTaggableOn::Tagging.table_name}.tag_id, COUNT(#{ActsAsTaggableOn::Tagging.table_name}.tag_id) AS tags_count")
tag_scope = ActsAsTaggableOn::Tag.select("#{ActsAsTaggableOn::Tag.table_name}.*, #{ActsAsTaggableOn::Tagging.table_name}.tags_count AS count").order(options[:order]).limit(options[:limit])
Expand Down Expand Up @@ -154,12 +152,12 @@ def tag_scope_joins(tag_scope, tagging_scope)
end

def tag_counts_on(context, options={})
self.class.tag_counts_on(context, options.merge(:id => id))
self.class.tag_counts_on(context, options.merge(id: id))
end

module CalculationMethods
def count(column_name = nil)
return super(column_name) if ActsAsTaggableOn::Utils.active_record42?
return super(column_name) if ActsAsTaggableOn::Utils.active_record4?
# https://github.com/rails/rails/commit/da9b5d4a8435b744fcf278fffd6d7f1e36d4a4f2
super(:all)
end
Expand Down
7 changes: 4 additions & 3 deletions lib/acts_as_taggable_on/acts_as_taggable_on/compatibility.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module ActsAsTaggableOn::Compatibility
def has_many_with_compatibility(name, options = {}, &extention)
if ActiveRecord::VERSION::MAJOR >= 4
if ActsAsTaggableOn::Utils.active_record4?
scope, opts = build_scope_and_options(options)
has_many(name, scope, opts, &extention)
else
Expand All @@ -13,11 +13,12 @@ def build_scope_and_options(opts)

unless scope_opts.empty?
scope = lambda do
scope_opts.inject(self) { |result, hash| result.send *hash }
scope_opts.inject(self) { |result, hash| result.send(*hash) }
end
return [scope, opts]
end

[defined?(scope) ? scope : nil, opts]
[nil,opts]
end

def parse_options(opts)
Expand Down
Loading

5 comments on commit 4c5314b

@ches
Copy link
Contributor

@ches ches commented on 4c5314b Apr 18, 2014

Choose a reason for hiding this comment

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

Hashrockets aren't deprecated, they are necessary for some legal hash keys for which the JS style doesn't parse (e.g. { Foo.new => "bar" }. Please try to consider that sweeping formatting/style commits like this are painful for some of us who are asked to rebase our 6 month-old pull requests.

@seuros
Copy link
Collaborator Author

@seuros seuros commented on 4c5314b Apr 18, 2014

Choose a reason for hiding this comment

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

since it my fault, i will do the rebasing for you. Is that fine for you ? if your repo don't have other changes, you will be able to use master.

@ches
Copy link
Contributor

@ches ches commented on 4c5314b Apr 18, 2014

Choose a reason for hiding this comment

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

@seuros No worries, thank you for the offer -- I've just finished rebasing #413, #411 should probably be pretty easy whenever that one gets reviewed again.

@bf4
Copy link
Collaborator

@bf4 bf4 commented on 4c5314b Apr 18, 2014

Choose a reason for hiding this comment

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

@ches +1. Would you mind being added as a maintainer or being called in to discussions, occasionally?

@ches
Copy link
Contributor

@ches ches commented on 4c5314b Apr 27, 2014

Choose a reason for hiding this comment

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

@bf4 Apologies for the slow response, I'm on holiday at the moment—sure, I think I've probably developed a deep enough familiarity with the code base at this point to contribute to discussion and make reasonable judgments. I've done work on some further pretty heavy changes that I'm not sure would ever be appropriate to merge into the release version or not, but they seem to be serving our purposes in production for awhile without problems FWIW.

Please sign in to comment.