This project is archived and is in readonly mode.

sexier migration that doesn't need the block parameter for create_table

#6339

This patch enables the following syntax for AR::TableDefinition#create_table.

class CreateUsers < ActiveRecord::Migration
  def change
    create_table :users do
      string :name
      timestamps
    end
  end
end

Traditional syntax is still available. So, this patch does not break any existing migration files.

class CreateUsers < ActiveRecord::Migration
  def change
    create_table :users do |t|
      t.string :name
      t.timestamps
    end
  end
end

I believe the new syntax is more Rails-3-ish, I mean, consistent Rails 3 API, since the routes DSL and ActionMailer DSL also dropped their block parameters in Rails 3.

And, I think 3.1 is the best timing to apply this change because some other changes were already introduced to the migration DSL.

sexier_migration.diff 5.91 KB

Reported by Akira Matsuda · January 26th, 2011 @ 11:52 PM

Activity

Rohit Arondekar
- Importance changed from to Low
I like it. It does fit in more with the Rails 3 way of doing things.

I'd love it if we can get more people to review the patch or give their opinions so that we can assign it to a core member for review. :)

January 27th, 2011 @ 03:44 AM
Jesse Storimer
I'm not core, but I think it's great.

January 27th, 2011 @ 04:01 AM
Prem Sichanugrist (sikachu)
I think I like this patch. It really fits the trend of Rails 3 as in the route, where we remove map. from the route file.

So yeah, +1. Would love to get this applied.

Aaron, can I have your opinion too?

January 27th, 2011 @ 04:28 AM
lakshmanan
+1

looks cool.

January 27th, 2011 @ 08:42 AM
Jan Jones
+1

January 27th, 2011 @ 08:47 AM
Rohit Arondekar
- State changed from new to open
- Assigned user set to José Valim
Ok, assigning to a core member for consideration. :)

January 27th, 2011 @ 12:27 PM
José Valim
- State changed from open to wontfix
Thanks for the patch, but I have -1.

Using instance_eval changes the self.scope and usually causes a lot of confusion. For that reason, Rails avoids using instance_eval whenever it can.

In a few places, like the router, it is convenient because we usually nest several blocks, one inside the other. But this is not the case for migrations. So I will stick with simplicity.

January 27th, 2011 @ 12:37 PM
Akira Matsuda
@José

Thanks for reviewing!
I think I understand your point. Yes, eval family should not be abused. It may cause unexpected weird problem when combined with other metaprogramming technique.
However, I think it won't cause any problem in this paricular case, because we're talking about migrations.

Methods we use inside the create_table block are besically restricted to the following 15 methods.
```
# You see this migration command surely runs on my console.
ActiveRecord::Schema.define do
  create_table :people do
    column :string, :col1
    timestamps
    references :model1
    belongs_to :model2
    string :col2
    text :col3
    integer :col4
    float :col4
    decimal :col5
    datetime :col6
    timestamp :col7
    time :col8
    date :col9
    binary :col10
    boolean :col11
  end
end

-- create_table(:people)
   -> 0.0060s
```
That is why instance_eval is already used in AR::Schema.define without causing any problem ever. https://github.com/rails/rails/blob/master/activerecord/lib/active_...
Who does metaprogramming inside create_table block in migration files?

So, what do you actually worry? Am I missing any possible usecase?
January 27th, 2011 @ 02:55 PM

Akira Matsuda

# sorry I made a tiny mistake
s/column :string, :col1/column :col1, :string/

January 27th, 2011 @ 03:04 PM

José Valim
- Assigned user changed from José Valim to Aaron Patterson
Ok, I am on the fence. :) Aaron Patterson is responsible for AR so I will defer the final decision to him.

January 27th, 2011 @ 03:41 PM
Matt Kern
+1

I'm for the simplification for sure. And, though I completely agree that instance_eval can be problematic, in this case it seems totally worthwhile.

May 20th, 2011 @ 04:44 AM