Skip to content
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

Implemented extra boolean param for rollback() #2968

Merged
merged 4 commits into from Dec 20, 2018

Conversation

edevil
Copy link
Contributor

@edevil edevil commented Dec 20, 2018

Extra parameter allows all migrations to be rolled back.
Addresses #2844

Extra parameter allows all migrations to be rolled back.
});

it('should delete all batches from the migration log', function() {
debugger;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably this shouldn't be committed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crap, sorry...

});
});

it('should delete all batches from the migration log', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use arrow function?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well yeah when it's needed, but when it's not?..

true
)
.spread(function(batchNo, log) {
console.log(`Batch ${batchNo}, log: ${JSON.stringify(log)}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry.

@@ -829,7 +829,7 @@ declare namespace Knex {
interface Migrator {
make(name: string, config?: MigratorConfig): Bluebird<string>;
latest(config?: MigratorConfig): Bluebird<any>;
rollback(config?: MigratorConfig): Bluebird<any>;
rollback(config?: MigratorConfig, all?: boolean): Bluebird<any>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha, nice touch!

@kibertoad
Copy link
Collaborator

Could you also create a PR in documentation repo?

@kibertoad
Copy link
Collaborator

LGTM, we can merge it after documentation is created.

@edevil
Copy link
Contributor Author

edevil commented Dec 20, 2018

@kibertoad Great! The docs PR is at knex/documentation#170

@kibertoad kibertoad merged commit c2dff7f into knex:master Dec 20, 2018
@kibertoad
Copy link
Collaborator

Thank you a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants