mattes / migrate

Database migrations. CLI and Golang library.
Other
2.29k stars 326 forks source link

Don't error if there are no up migrations #287

Open tjarratt opened 7 years ago

tjarratt commented 7 years ago

Basically a dupe of #250 but I really wanted to re-open this conversation.

Is it really true that there are use cases of Migrate.Up() or Migrate.Steps(n int)` that really intend to fail if there are no changes? Most consumers I could imagine would not expect these methods to return an error when there is nothing to migrate - they want to run any migrations that exist, and know only if there were problems running new migrations.

This API effectively forces every single caller to check yet another possible error, for something that is not really an error 99.9% of the time. If I have a webserver that uses mattes/migrate, adding a new migration isn't a terribly common action, so my application is not expecting Migrate.Up() to return an error for an exceptional case.

Sorry to complain so much, but it was really frustrating to encounter this after a night debugging the mysql driver, and this was really just the icing on top of my own personal cake. For what it's worth, I found all of the other aspects of using this package to be really delightful - this is really my only gripe from an otherwise painless integration.

Thanks for making (and continuing to maintain) this package!

mattes commented 7 years ago

Thanks for your feedback. I'm open to changing it. A lot of people ran into this.

GeertJohan commented 6 years ago

Changing the API to not return an error will break people's code where they actually expect to get an error if there are no new migrations. I believe the current behaviour where it returns migrate.ErrNoChange is correct. (It could maybe be explained clearer in the Up() docs though)

dotchev commented 6 years ago

Strictly speaking this is not an error. If it is necessary to communicate if the db was changed, a separate return value could be used, e.g. Up() (stepsExecuted int, err error) of course any change in the API would be incompatible, so probably for the next major version, maybe in https://github.com/golang-migrate/migrate