Refactored the migrate code and fixed minor bugs. #80

Merged
merged 3 commits into from Oct 19, 2012

Projects

None yet

3 participants

@thanosp

Added immediate return calls when the user decides not to go through with the migration, right after a message is printed.
On the first case(unregistered migrations) input was just ignored.
On the second $sql was never set.
Cleaned up the nested if statements to make the code more readable and maintainable.

@thanosp thanosp Refactored the migrate code to make it more readable.
Added immediate stopping when the user decides not to go through with the migration.
a4171c4
@stof stof commented on an outdated diff Jul 5, 2012
...L/Migrations/Tools/Console/Command/MigrateCommand.php
@@ -96,6 +96,10 @@ public function execute(InputInterface $input, OutputInterface $output)
if ($noInteraction === false) {
$confirmation = $this->getHelper('dialog')->askConfirmation($output, '<question>Are you sure you wish to continue? (y/n)</question>', false);
+ if ($confirmation !== true) {
@stof
stof Jul 5, 2012

if ( ! $confirmation) {

@thanosp

How about now? Corrected a bit more than the confirmation variable.

@thanosp

So.....

@stloyd

@beberlei @jwage Anyone ? It's quite long time this waits... Also there are 4 (two mentioned above and #78 + #93) other PRs that can be closed as duplicates after this one is merged.

@stof stof commented on an outdated diff Oct 18, 2012
...L/Migrations/Tools/Console/Command/MigrateCommand.php
$confirmation = $this->getHelper('dialog')->askConfirmation($output, '<question>Are you sure you wish to continue? (y/n)</question>', false);
+ if ( ! $confirmation) {
+ $output->writeln('<error>Migration cancelled!</error>');
+ return;
@stof
stof Oct 18, 2012

you should return 1 here to set the appropriate exit code

@stof stof commented on an outdated diff Oct 18, 2012
...L/Migrations/Tools/Console/Command/MigrateCommand.php
- if ($noInteraction === true) {
- $sql = $migration->migrate($version, $dryRun);
- } else {
- $confirmation = $this->getHelper('dialog')->askConfirmation($output, '<question>WARNING! You are about to execute a database migration that could result in schema changes and data lost. Are you sure you wish to continue? (y/n)</question>', false);
- if ($confirmation === true) {
- $sql = $migration->migrate($version, $dryRun);
- } else {
- $output->writeln('<error>Migration cancelled!</error>');
- }
+
+ // warn the user if no dry run and interaction is on
+ if ( ! $dryRun && ! $noInteraction) {
+ $confirmation = $this->getHelper('dialog')->askConfirmation($output, '<question>WARNING! You are about to execute a database migration that could result in schema changes and data lost. Are you sure you wish to continue? (y/n)</question>', false);
+ if ( ! $confirmation) {
+ $output->writeln('<error>Migration cancelled!</error>');
+ return;
@thanosp

Done

@stof stof merged commit ebee346 into doctrine:master Oct 19, 2012

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment