Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Remove useless argument #338

Closed
wants to merge 1 commit into from

8 participants

Sebastian Martinez Henrik Hodne Aaron Patterson José Valim David Chelimsky Jeremy Kemper Rafael Mendonça França Carlos Antonio da Silva
Sebastian Martinez

The +name+ argument on the #columns method is useless, no implementation of this method makes use of it.
I removed the argument since it only creates confusion.

Henrik Hodne

SQLite 3 tests pass.

Aaron Patterson
Owner

I'd like to avoid changing method signatures until Rails 4.0. This change will break backwards compatibility with 3rd party adapters.

José Valim
Owner

@tenderlove should we leave this open then? or can we close?

Henrik Hodne

Set up a milestone and add it?

Aaron Patterson
Owner

Yes, we should set up a rails 4 milestone and add this.

David Chelimsky

How about changing its name to something like deprecated for rails 3-1 so people know it's going to go away? That would also minimize the confusion that @smartinez87 cites.

Jeremy Kemper
Owner

Milestoned @ Rails 4.0

Sebastian Martinez

master is rails 4 now right? should this be merged?

Rafael Mendonça França
Owner

Yeah! Master is Rails 4.0 :heart:

Carlos Antonio da Silva

@smartinez87 I think it'd be good to rebase so it can be merged now :)

Sebastian Martinez

had trouble rebasing master again, so opened a new PR #4850

Sebastian Martinez

closing this, since #4850 was merged

Sebastian Martinez smartinez87 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 28, 2011
  1. Sebastian Martinez
This page is out of date. Refresh to see the latest.
2  activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
View
@@ -90,7 +90,7 @@ def initialize(spec)
h[table_name] = with_connection do |conn|
# Fetch a list of columns
- conn.columns(table_name, "#{table_name} Columns").tap do |columns|
+ conn.columns(table_name).tap do |columns|
# set primary key information
columns.each do |column|
2  activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
View
@@ -50,7 +50,7 @@ def index_exists?(table_name, column_name, options = {})
# Returns an array of Column objects for the table specified by +table_name+.
# See the concrete implementation for details on the expected parameter values.
- def columns(table_name, name = nil) end
+ def columns(table_name) end
# Checks to see if a column exists in a given table.
#
2  activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
View
@@ -424,7 +424,7 @@ def indexes(table_name, name = nil)
indexes
end
- def columns(table_name, name = nil)
+ def columns(table_name)
sql = "SHOW FIELDS FROM #{quote_table_name(table_name)}"
columns = []
result = execute(sql)
2  activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
View
@@ -625,7 +625,7 @@ def indexes(table_name, name = nil)#:nodoc:
indexes
end
- def columns(table_name, name = nil)#:nodoc:
+ def columns(table_name)#:nodoc:
sql = "SHOW FIELDS FROM #{quote_table_name(table_name)}"
columns = []
result = execute(sql, 'SCHEMA')
2  activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
View
@@ -734,7 +734,7 @@ def indexes(table_name, name = nil)
end
# Returns the list of all column definitions for a table.
- def columns(table_name, name = nil)
+ def columns(table_name)
# Limit, precision, and scale are all handled by the superclass.
column_definitions(table_name).collect do |column_name, type, default, notnull|
PostgreSQLColumn.new(column_name, default, type, notnull == 'f')
2  activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
View
@@ -244,7 +244,7 @@ def tables(name = 'SCHEMA') #:nodoc:
end
end
- def columns(table_name, name = nil) #:nodoc:
+ def columns(table_name) #:nodoc:
table_structure(table_name).map do |field|
case field["dflt_value"]
when /^null$/i
4 activerecord/lib/active_record/reflection.rb
View
@@ -242,8 +242,8 @@ def counter_cache_column
end
end
- def columns(tbl_name, log_msg)
- @columns ||= klass.connection.columns(tbl_name, log_msg)
+ def columns(tbl_name)
+ @columns ||= klass.connection.columns(tbl_name)
end
def reset_column_information
2  activerecord/test/active_record/connection_adapters/fake_adapter.rb
View
@@ -28,7 +28,7 @@ def merge_column(table_name, name, sql_type = nil, options = {})
options[:null])
end
- def columns(table_name, message)
+ def columns(table_name)
@columns[table_name]
end
end
9 activerecord/test/cases/migration_test.rb
View
@@ -905,20 +905,19 @@ def test_rename_table_with_an_index
def test_change_column
Person.connection.add_column 'people', 'age', :integer
- label = "test_change_column Columns"
- old_columns = Person.connection.columns(Person.table_name, label)
+ old_columns = Person.connection.columns(Person.table_name)
assert old_columns.find { |c| c.name == 'age' and c.type == :integer }
assert_nothing_raised { Person.connection.change_column "people", "age", :string }
- new_columns = Person.connection.columns(Person.table_name, label)
+ new_columns = Person.connection.columns(Person.table_name)
assert_nil new_columns.find { |c| c.name == 'age' and c.type == :integer }
assert new_columns.find { |c| c.name == 'age' and c.type == :string }
- old_columns = Topic.connection.columns(Topic.table_name, label)
+ old_columns = Topic.connection.columns(Topic.table_name)
assert old_columns.find { |c| c.name == 'approved' and c.type == :boolean and c.default == true }
assert_nothing_raised { Topic.connection.change_column :topics, :approved, :boolean, :default => false }
- new_columns = Topic.connection.columns(Topic.table_name, label)
+ new_columns = Topic.connection.columns(Topic.table_name)
assert_nil new_columns.find { |c| c.name == 'approved' and c.type == :boolean and c.default == true }
assert new_columns.find { |c| c.name == 'approved' and c.type == :boolean and c.default == false }
assert_nothing_raised { Topic.connection.change_column :topics, :approved, :boolean, :default => true }
Something went wrong with that request. Please try again.