This project is archived and is in readonly mode.
sexier migration that doesn't need the block parameter for create_table
Reported by Akira Matsuda | January 26th, 2011 @ 11:52 PM
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.
Comments and changes to this ticket
-
Rohit Arondekar January 27th, 2011 @ 03:44 AM
- 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. :)
-
Prem Sichanugrist (sikachu) January 27th, 2011 @ 04:28 AM
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?
-
Rohit Arondekar January 27th, 2011 @ 12:27 PM
- State changed from new to open
- Assigned user set to José Valim
Ok, assigning to a core member for consideration. :)
-
José Valim January 27th, 2011 @ 12:37 PM
- 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.
-
Akira Matsuda January 27th, 2011 @ 02:55 PM
@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?
-
Akira Matsuda January 27th, 2011 @ 03:04 PM
# sorry I made a tiny mistake s/column :string, :col1/column :col1, :string/
-
José Valim January 27th, 2011 @ 03:41 PM
- 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.
-
Matt Kern May 20th, 2011 @ 04:44 AM
+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.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>