Add callback to run #547
Add callback to run #547
Conversation
|
There were the following issues with this Pull Request
You may need to change the commit messages to comply with the repository contributing guidelines. 🤖 This comment was generated by commitlint[bot]. Please report issues here. Happy coding! |
Signed-off-by: cellis <cameronellis.ellis@gmail.com>
|
There were the following issues with this Pull Request
You may need to change the commit messages to comply with the repository contributing guidelines. 🤖 This comment was generated by commitlint[bot]. Please report issues here. Happy coding! |
|
As stated in the issue. Not gonna happen, run is explicitly not returning a callback and never will. It will exit with proper exit codes and is CLI only. I have however still reviewed your changes, so you know for your next PR what to look out for. |
| var action = internals.argv._.shift(); | ||
| var folder = action.split(':'); | ||
|
|
||
| action = folder[0]; | ||
| var toExecute = null; |
There was a problem hiding this comment.
always move definitions to the top of the current scope
There was a problem hiding this comment.
also do not initialize something with null just leave it alone when you want to have it published for the scope but filled later.
| @@ -4,12 +4,14 @@ var log = require('db-migrate-shared').log; | |||
| var optimist = require('optimist'); | |||
| var load = require('./'); | |||
| var transition = load('transition'); | |||
| Promise = require('bluebird'); | |||
| run: function () { | ||
| load('run')(this.internals, this.config); | ||
| run: function (callback) { | ||
| load('run')(this.internals, this.config, callback); |
There was a problem hiding this comment.
not a valid definition, everything needs to be a promise in the first place. callbacks are just second class citizens and just supported for backwards compatibility.
This makes it much simpler to hook standard CLI behavior with custom behavior, as detailed in the docs: https://db-migrate.readthedocs.io/en/latest/API/programable/#run
Related: #313