cakephp / phinx

PHP Database Migrations for Everyone
https://phinx.org
MIT License
4.46k stars 894 forks source link

ability to configure the migration base class? #314

Closed iamjochem closed 9 years ago

iamjochem commented 9 years ago

I would really like to be able to have all my migration classes extend a custom class (rather than extend AbstractMigration directly) ... it would be nice if it was possible to specify an FQCN for the "migration-base-class" ... this implies that either the migration class template file be customizable (or that the template is changed so that the "base class" name is paramterized, rather than being hard-coded as it is now).

\ why do I want a custom base class for migrations? **

I want to define a number of custom reusable methods for running queries that are defined in seperate *.sql files - when a single migration requires changing/creating/deleting multiple stored procedures you start to appreciate the ability to define these "long" queries in seperate SQL files (which your awesome editor/IDE obviously syntax highlights) ... I would also like to be able to re-run (or undo) certain queries from previous migrations (to avoid duplication and/or copy-paste errors in migrations)

@robmorgan - what is you stance on the idea of being to configure an alternative migration base class?

  1. go away, your idea sucks the big one.
  2. nice, gimme a PR and I'll consider it (given the number of open PRs I'd question whether it would ever get considered/merged ... I'm assuming you have just 2 hands and are without a time-machine :-))
  3. best idea ever, I implemented it straight away, please pull master.

thanks for reading!

ghost commented 9 years ago

The idea behind migrations would go against nit picking undoing of previous queries in what you are proposing in a base class. This would take away of having explicit verbose migrations that would allow back tracing of schema changes and the state and growth of a application of a given timeline.

iamjochem commented 9 years ago

hi @BardiaAfshin,

to be honest I don't fully understand what it is you are trying to say, but ... you're point about "nit picking undoing of previous queries" seems to me to describe Phinx's change() method pretty well. to be clear, I think the change() method is quite a nice little shortcut method.

A migration, as I see it is merely a self-contained definition of a "state change" in two directions (the "up" and the "down"), how any given migration implements this is not relevant, i.e. it is not important whether I happen to implement a given migrations' "up" by reusing some SQL/query/migration/elephant, but merely that the migration "works"

For my project I happen to want to define some custom helper methods for use in my migrations and the nicest way to do that it is being able to tell Phinx which migration class I want to use when generating a new migration file (as long as the specified class implements the required interface there should be no problem, in practice extending the AbstractMigration class is the easiest way too go about that) ... there are many other ways to get the same result but that all entail having to manually add/remove/change [boilerplate] code in each generated migration file ... that is somewhat tedious and error-prone.

I am not asking for any new functionality to be added to Phinx, I'm merely looking for the ability to specify (in the phinx config) a different class than the hardcoded "AbstractMigration" so that when I run this:

$ phinx create MyNewMigration

I get a file containing this:

<?php

use Phinx\Migration\AbstractMigration;

class MyNewMigration extends \MyApp\SomeNamespace\CrazyAbstractMigration
{
    /**
     * Migrate Up.
     */
    public function up()
    {

    }

    /**
     * Migrate Down.
     */
    public function down()
    {

    }
}
ghost commented 9 years ago

I see, I have no counter argument

iamjochem commented 9 years ago

ah cool - glad I managed to explain thing better the second time around, and thanks for taking the time to read!

cbrunnkvist commented 9 years ago

I had to access a few constants (DB_TABLE_PREFIX etc) which normally are loaded from the app config: I got that working by using phinx.php instead of phinx.yml.

That way you get a logical place to "hook in" and load whatever helpers you may need later on inside migrations, without having to add require/include/use everywhere. :bulb:

iamjochem commented 9 years ago

@cbrunnkvist - that implies either using a custom wrapper script to load said functionality or hacking phinx.php itself, I don't think either is particularly clean (ideally devs would just be able to use the phinx command out-of-the-box, that way nobody will forget to use the wrapper and/or add a hacked version of phinx to a project).

regardless, #361 would be the solution to my problem if it gets merged.

lorenzo commented 9 years ago

I think the ability of specifying a custom template for the migration class is more valuable and solves more problems than being able to only specify the base migration class

cbrunnkvist commented 9 years ago

@iamjochem I'm not sure you see what I mean: I'm not talking about any wrapper or the actual implementation file src/.../phinx.php: what I meant is, there is a (somewhat undocumented?) feature of Phinx that by default tries several configuration formats:

...the latter of which can be used for preparing any globals/environment you need for the migrations. The only contract is that including it must return an array representing the config.

Ref: https://github.com/robmorgan/phinx/blob/0.3.x-dev/src/Phinx/Config/Config.php#L105

iamjochem commented 9 years ago

@cbrunnkvist - ah I see, indeed that is much better that what I had assumed you meant! thanks for clarifying