Right now, if you accidentally give a migration filename an upper-case character (say, 001-initialMigration.rb), you get a crash:
You have a nil object when you didn’t expect it!
You might have expected an instance of Array.
The error occurred while evaluating nil.first
/home/bronson/workspace-hobo/hobo-docs/vendor/rails/activerecord/lib/active_record/migration.rb:367:in `migration_files’
I would like to fix this.
Isn’t this an artificial limitation? Why not just add “A-Z” to the regex in migration.rb so that migration names can contain capitals?
def migration_version_and_name(migration_file)
Either way, should I submit a patch that does something like this?
(and, of course, create the IllegalMigrationNameError object)
That way, at least, Rails will spit out an intelligible error message instead of bombing out when it calls nil.first.
Does this make sense?
- Scott
Right now, if you accidentally give a migration filename an upper-case
character (say, 001-initialMigration.rb), you get a crash:
You have a nil object when you didn't expect it!
You might have expected an instance of Array.
The error occurred while evaluating nil.first
/home/bronson/workspace-hobo/hobo-docs/vendor/rails/activerecord/lib/active_record/migration.rb:367:in
`migration_files'
I would like to fix this.
Isn't this an artificial limitation? Why not just add "A-Z" to the regex in
migration.rb so that migration names can contain capitals?
def migration_version_and_name(migration_file)
- return
*migration_file.scan(/([0-9]+)_([_a-z0-9]*).rb/).first
+ return
*migration_file.scan(/([0-9]+)_([_A-Za-z0-9]*).rb/).first
end
I wrote about this some time ago:
http://talklikeaduck.denhaven2.com/articles/2007/09/11/conventions-uber-alles
prompted by this, Ben Scofield proposed a similar patch:
http://dev.rubyonrails.org/ticket/9542
Which was rejected because it would violate roundtrips from classname
to filename.
Either way, should I submit a patch that does something like this?
- migration_version_and_name(f).first.to_i
+ m = migration_version_and_name(f)
+ raise IllegalMigrationNameError.new(f) if m.nil?
+ m.first.to_i
(and, of course, create the IllegalMigrationNameError object)
That way, at least, Rails will spit out an intelligible error message
instead of bombing out when it calls nil.first.
Well why not submit it and see what the maintainers have to say.
Right now, if you accidentally give a migration filename an upper-case character (say, 001-initialMigration.rb), you get a crash:
Was this file made from the migration generator or by hand? It seems like we should fix this if it was somehow generated improperly but hand-written migrations should probably need to follow convention.
Either way, should I submit a patch that does something like this?
.first.to_i
(and, of course, create the IllegalMigrationNameError object)
That way, at least, Rails will spit out an intelligible error message instead of bombing out when it calls nil.first.
That’s a great idea. Well-named errors (especially on the command line) are a real help to new Railers (and old ones too).
::Jack Danger
prompted by this, Ben Scofield proposed a similar patch:
http://dev.rubyonrails.org/ticket/9542
Which was rejected because it would violate roundtrips from classname
to filename.
That makes total sense. Given that, I agree that migrations filenames should be restricted to lower case.
Was this file made from the migration generator or by hand? It seems like we should fix this if it was somehow generated improperly but hand-written migrations should probably need to follow convention.
It was made using the Hobo migration generator which apparently doesn’t sanitize the filenames. I’ll open a Hobo ticket.
Either way, should I submit a patch…?
That’s a great idea. Well-named errors (especially on the command line) are a real help to new Railers (and old ones too).
Ticket: http://dev.rubyonrails.org/ticket/9909
Please vote for it!
- Scott