feat: Add method to run migration script by name. #12
Conversation
| * @param {String} [record] Record the migration runtime to database. | ||
| * @param {Function} [cb] Callback function. | ||
| */ | ||
| Migration.migrateByName = function(name, record, cb) { |
There was a problem hiding this comment.
I think this line should be removed @rahulbile as I don't think it makes sense to default the name to an empty string.
lib/models/migration.js
Outdated
| * @param {Function} [cb] Callback function. | ||
| */ | ||
| Migration.migrateByName = function(name, record, cb) { | ||
| name = name || ''; |
There was a problem hiding this comment.
I think record should be an optional property - you need to add something like this just above this line:
if (typeof cb === 'undefined' && typeof record === 'function') {
cb = record
record = false
}
lib/models/migration.js
Outdated
| try { | ||
| require(scriptPath).up(Migration.app, function(err) { | ||
| if (err) { | ||
| Migration.log.error(name, 'error:'); |
There was a problem hiding this comment.
I'd suggest combining these 2 lines so that only one error message is logged if there is an error. Perhaps make the error message a little more descriptive too:
Migration.log.error(`Error running migration script ${name}:`, err);
| Migration.create({ | ||
| name: name, | ||
| runDtTm: new Date() | ||
| }); |
There was a problem hiding this comment.
You're missing a call to cb() after a successful migration.
lib/models/migration.js
Outdated
| } | ||
| }); | ||
| } catch (err) { | ||
| Migration.log.error('Error running migration', name); |
There was a problem hiding this comment.
Lets combine these 2 lines into a single log message too @rahulbile
| Migration.create({ | ||
| name: name, | ||
| runDtTm: new Date() | ||
| }); |
There was a problem hiding this comment.
I think this needs to move to below this if block since we want to call it regardless of the value of record
2 participants
No description provided.