Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

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

Open
nesrual opened this issue Mar 25, 2016 · 8 comments
Open

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

nesrual opened this issue Mar 25, 2016 · 8 comments

Comments

@nesrual
Copy link

nesrual commented Mar 25, 2016

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
Copy link
Member

ronen commented Mar 28, 2016

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
Copy link
Member

fj commented Mar 28, 2016

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
Copy link
Member

ronen commented Mar 28, 2016

@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
Copy link
Author

nesrual commented Mar 29, 2016

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 👍

@fj
Copy link
Member

fj commented May 9, 2016

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:

  • {"!foo": 1}: key is the symbol :"!foo"
  • {:"!foo" => 1}: key is the symbol :"!foo"
  • {"!foo" => 1}: key is the string "!foo"

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

@fj
Copy link
Member

fj commented May 9, 2016

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
Copy link

FWIW, 👎

@ronen
Copy link
Member

ronen commented May 16, 2016

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants