-
-
Notifications
You must be signed in to change notification settings - Fork 526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add db:migrate:status #385
Conversation
return ensureCurrentMetaSchema(migrator).then(function () { | ||
return migrator.executed(); | ||
}).then(function (migrations) { | ||
_.forEach(migrations, function (migration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the way you are logging the results. Maybe something like this would be nicer:
Executed migrations:
- <migration file 1>
- <migration file 2>
...
Pending migrations:
- <migration file 3>
- <migration file 4>
...
If there are no executed or pending migrations, instead of that just log:
No executed migrations found.
No pending migrations found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format I chose was modeled after ActiveRecord's 'rake db:migrate:status', which will make it familiar to many users.
Having the 'up' or 'down' at the front of each line makes it easy to grep
for any down migrations.
Adding the extra verbiage if there are no migrations of a particular type goes against the typical unix command-line style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your point here, it is helpful for chaining more commands on the output.
@Americas Either one of these PRs will be fine with me. I'm mostly interested in seeing down migrations. |
ActiveRecord has Following their precedent of two separate commands, instead of one with options makes sense to me. It also makes for a nice parallel between the two migrations systems, which seems valuable. |
@Americas @sdepold @cusspvz The question of "Should we create 2 commands or just add options to this one so it errors if there are pending migrations?" can be handled over there. Can this get merged? Thanks, |
Use go-update and semver for a better upgrade experience
This also addresses #165 , with a slightly different approach from #379.
Both
db:migrate:status
anddb:migrate:pending
would probably be useful.