Fixed #4295 #4308
Fixed #4295 #4308
Conversation
|
Thank you! |
| !migrations.some((migration) => { | ||
| const migrationContents = this.config.migrationSource.getMigration( | ||
| migration | ||
| ); | ||
| return !this._useTransaction(migrationContents); | ||
| }); | ||
| !( | ||
| await Promise.all( | ||
| migrations.map(async (migration) => { | ||
| const migrationContents = await this.config.migrationSource.getMigration( | ||
| migration | ||
| ); | ||
| return !this._useTransaction(migrationContents); | ||
| }) | ||
| ) | ||
| ).some((isTransactionUsed) => isTransactionUsed); |
There was a problem hiding this comment.
were the changes in Migrator intended in this PR? what issue are they addressing?
There was a problem hiding this comment.
Okay I can see more details in #4296
There was a problem hiding this comment.
@falkenhawk Did that cause some breaking changes for you?
There was a problem hiding this comment.
I can't tell yet, I noticed this while I was looking through the changes after I got notified by renovatebot about the new knex version 0.21.18... and not having this explicitly listed in changelog confused me and stopped me from upgrading, as it seemed to me to be something extra than just a fix for #4295 - and I could not afford breaking things at that moment 😅
If I decide to upgrade in the following days and will run into some issues, I will let you know then.
There was a problem hiding this comment.
It should behave same way for the end user in most cases, and also handle ESM case better, see #4296 (comment) for the reason of more methods becoming async. If you are not using ESM, this upgrade won't do anything useful for you.
Backport of #4296