Suggestion: Throw `IrreversibleMigrationException` also when migrating up

[Taitava]

Hi,

a quote from the docs that describe the change() method (scroll down a little bit after clicking the link):

If a command cannot be reversed then Phinx will throw an IrreversibleMigrationException when it’s migrating down.

As I usually only run migrations up, it might be that I will not notice if I happen to create migrations that accidentally use some irreversible actions in the change() method. Could it be possible to make Phinx throw this exception also when migrating up, so it would be easier to spot these mistakes earlier? Maybe this behaviour could be toggled on/off by a setting in phinx.yml?

This would reduce a risk that a "downwards broken" migration would end up to be used in a production deployment process which for some reason needs to be rolled back, and then the rollback would fail due to a mistake that was not spotted in the development phase, causing that the mistake would have to be fixed by manually editing the migration file on a production server and usually in a big hurry. :D

Thanks! :)

[dereuromark]

Do you want to make a PR here?

[Taitava]

I'd kind of like to make a PR, but I think we need to plan it a bit first. First of all, I'm very new to this tool and I might not be aware of all the things which this might affect. But then again this could be a very good opportunity for me to get to know phinx better 'under the hood'.

One thing that I've come to notice after opening this suggestion is that if this were to be implemented, there must be a clean way to prevent throwing these exceptions in some special cases. Please take a look at the following example which creates a new table:

class migration extends AbstractMigration
{
    public function change()
    {
        // Migrating up or down:
        $this->table('my_table')->addColumn('my_column','string')->create();

        // Only when migrating up:
        if ($this->isMigratingUp())
        {
            $this->table('my_table')->insertData(*some data*); // We would need to ensure that no `IrreversibleMigrationException` will be thrown when calling non reversible commands in a scope which is clearly never reachable during rollbacks.
        }
    }
}

Any ideas about this? :)

[boenrobot]

How about a command line flag / config setting, which, if turned on, will immediately run the migration down, and up again, thus ensuring it is reversible.

I understand that some migrations might take a while to run, especially on prod environments, so this behaviour should be opt in. I.e. by default, migrations run only in the specified direction. Me personally, I would only run such a mode on a local copy, where due to the relatively small amount of data, even the otherwise super slow migrations would run pretty much instantly.

[Taitava]

Hi @boenrobot, it might be one solution, considering the issue that I brought up in my previous comment on 2019-12-08. Your suggestion would solve that problem.

First I thought that it would be a bit redundant to run a migration three times (first up, then down, and then up again). But now that I have thought about it for a moment, it might make sense: It ensures that also redoing a migration (=doing the last up operation) works correctly after a down operation. I think it might result in more "robust" migrations, because of increased amount of testing.

Edit: And I would suggest it to be a config setting, so that I can make it permanently happen every time on my development machine (and never do it in production). A command line flag might be a good additional feature, but in my opinion it would be nice if we would anyway have a config option.