feat: export Migration Execution API from main package (fixes #4880) #4892
Conversation
| }) | ||
| } | ||
|
|
||
| public async getAllMigrations(): Promise<Migration[]> { |
There was a problem hiding this comment.
why do we need it if we already have getMigrations ?
There was a problem hiding this comment.
getMigrations is a protected method where this one is part of the public API proposed returning a Promise<Mutation[]>, like its sibling methods getExecutedMigrations and getPendingMigrations.
There was a problem hiding this comment.
not sure about it. Not sure we really need this "all" name. But we can left it as you did.
There was a problem hiding this comment.
I just thought that for consistency they should all return promises instead and I didn't want to introduce any changes to current behaviour. I'll change getMigrations to return a Promise and make it public, if that's ok.
There was a problem hiding this comment.
choose any approach you prefer
|
Think we can merge it.. but tests are failing because of tslint errors, can you please fix them |
Yeah I can fix it, I was hoping to get some feedback first, because there are some other changes in the original issue which needs to be done, if they make any sense. |
|
For me those methods don't make sense since I never had such use case. But we still can make some methods public if users need it. I would like to hear other people opinions around this question but looks like nobody is interested |
leonardofalk
force-pushed
the
migration-api
branch
from
632a132
to
ad13069
Compare
leonardofalk
force-pushed
the
migration-api
branch
from
ad13069
to
b1238b5
Compare
Done. |
This may need a lot of rework and review, but it describes what I tried to explain in #4880