SchemaPlus / schema_plus

SchemaPlus provides a collection of enhancements and extensions to ActiveRecord
Other
679 stars 84 forks source link

Upgrade from 2.0.0 to 2.0.1 alters schema.db file format #231

Open nesrual opened 8 years ago

nesrual commented 8 years ago

Upgrading schema_plus and running any pending migrations will result in a changed db/schema.rb file:

#> bundle update schema_plus
#> rake db:migrate

The changed db/schema.rb file will use the old Ruby format where

t.string   "description",         default: "", null: false

will become

t.string   "description",         :default=>"", :null=>false
ronen commented 8 years ago

Yes this change is a byproduct of other changes. I didn't consider it a regression or breaking change since it's functionally equivalent.

TL;DR: the simplest cleanest way that I know to print a ruby hash that contains only primitives, in a form that can be read back in as valid ruby, is simply to use the builtin Hash#to_s -- which prints using the old Ruby format. Could write a custom dumper in some form but that seems like extra code (with more opportunities for bugs) for little functional benefit.

fj commented 8 years ago

This seems reasonable to me too.

@nesrual, is this breaking something specific for you? If the behavior of schema_plus changed as a result of the functionally equivalent hash changing, that's a bug. Otherwise this isn't really any different than if schema_plus decided to write:

t.string \
  "foo"

instead of

t.string "foo"
ronen commented 8 years ago

@nerusal I should add that I was disappointed by this change too, since the modern style makes for clear looking output IMHO. But I couldn't justify to myself muddying the code (and taking the time) just to make the output arguably prettier.

But of course as @fj commented, if the change in output format really does break something then of course it'd need to be fixed.

nesrual commented 8 years ago

Besides the output change in the schema.rb file all works fine - so not really a "bug" in that sense. However it would be nice to keep the format of the file in the "proper" format :+1:

fj commented 8 years ago

On further reflection, I think hash-rocket may actually be preferred. Unusual but theoretically legal database column names or property values (e.g. "!!!X&&&") might not also be legal Ruby symbol names (e.g. you can't write !!!X&&&: false), so you would have to quote everything, which is what the Hash.to_s method that @ronen currently implements uses.

But if you quote everything and decide to use the new-style Hash syntax, then there is an ambiguity between these three choices:

So I think not using hashrockets as the canonical representation would actually be a bug.

fj commented 8 years ago

BTW, if you want to keep your migrations "clean", one thing I've done is perform an empty migration, commit the change, and then continue with my subsequent migrations. That isolates the commit with the schema_plus syntax changes so that the rest of your commits are clean.

braindev commented 8 years ago

FWIW, 👎

ronen commented 8 years ago

@fj re needing hash-rocket for non-symbol keys -- I don't think that's a huge issue here, since the hash is used for column options, which are defined by reasonable humans to have reasonable names. Conceivably some nested values for some options could be arbitrary hashes though, I suppose. But still, a pretty output formatter could use symbol-colon form whenever the key is a symbol, and revert to hash-rocket only when necessary.

@braindev FWIW I'm willing to accept a PR that prettifies the output (with sufficient tests of course!), just don't have the time to do it myself right now. or perhaps following SOC, maybe somebody could write or find a simple gem that will do this, and schema_plus_core could use it.