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

Rollbacks by start time #1045

Merged
merged 12 commits into from Feb 28, 2017
Merged

Rollbacks by start time #1045

merged 12 commits into from Feb 28, 2017

Conversation

ghost
Copy link

@ghost ghost commented Feb 27, 2017

A new Configuration option named version_order was added. Its possible values are:

  • creation (the default): causes the Rollback and Status commands to order the executed migrations by their Creation Times (aka Migration IDs or Names).
  • execution: causes the Rollback and Status commands to order the executed migrations by their Execution Times (aka Start Times).

This means that when invoking the Rollback command:

  • With an execution version order, the last executed migration will be rollbacked.
  • With an execution version order and a -d (date) option, the migration start times will be used to determine the target version to rollback to. In other words, all migrations which were executed after that date will be rollbacked.
  • With an execution version order and a -t (target version) option, all migrations which were executed after the target version was executed will be rollbacked.

In terms of testing:

  • The old testRollbacksByDate test was renamed to testRollbackToDateTime and turned into a unit test, ensuring the rollback method is called with the right target version. A new testRollback was added which ensures the correct output of the rollback operation (as was being done in the old testRollbacksByDate test).
  • More test cases were added.
  • General test improvements were done (like adding the expected arguments to the underlying Manager calls in RollbackTest and StatusTest.

Other notes:

  • The old rollback and rollbackToDateTime methods were merged into a single rollback method that handles all cases and version orders.
  • This PR replaces Rollbacks with Version Order #926.

Daniel Gomes added 8 commits February 27, 2017 10:50
The old testRollbacksByDate test was renamed to testRollbackToDateTime and
turned into a unit test, ensuring the rollback method is called with the
right version. A new testRollback was added which ensures the correct
output of the rollback operation (as was being done in the old
testRollbacksByDate test).
More test cases were added.
* A new version_order configuration option was added.
* A new Config.getVersionOrder() and Config.isVersionOrderCreationTime()
were added.
* The getVersionLog() method now returns the versions indexed by
version creation time but already in the proper order, according
to the configuration version order.
* The Manage's "rollback" and "rollbackToDateTime" methods were
merged into  one (simply named "rollback").
* The new Manager.rollback method was adapted so that it serves any
combination of start/creation time ordering and target version/date.
* The Rollback command now "fills" in missing parts of target dates.
* Tests improved: now using test input/output from setUp.
* Tests improved: the expectations for the underlying Manager calls
now include the expected arguments.
* It now highlights the column the migrations are being ordered by.
* Down migrations are now showing up in the right place.
Code that uses the version order value now also has a default clause to
raise an exception in case of invalid values.
@ghost ghost mentioned this pull request Feb 27, 2017
@rquadling
Copy link
Collaborator

@daniel-gomes-sociomantic Thank you for that. Can you look at PHPStan's output at https://travis-ci.org/robmorgan/phinx/jobs/205745493#L348 and fix accordingly.

https://github.com/phpstan/phpstan is a static analysis tool which helps identify potential problems or inconsistent usages. VERY useful!!!

@ghost
Copy link
Author

ghost commented Feb 27, 2017

there ya go

@rquadling rquadling merged commit 9ab56b9 into cakephp:master Feb 28, 2017
@rquadling
Copy link
Collaborator

Finally. A massive thank you for sticking with this/us/me on this.

@ghost
Copy link
Author

ghost commented Mar 1, 2017

No problem! Which release is this gonna be in, then?

@rquadling
Copy link
Collaborator

rquadling commented Mar 1, 2017

@rquadling
Copy link
Collaborator

Considering your understanding of Phinx, I'd really like your input on #1044 and #1046.

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

Successfully merging this pull request may close these issues.

1 participant