cakephp / phinx

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

Misleading config file properties naming #729

Closed jnardiello closed 1 year ago

jnardiello commented 8 years ago

Hello everyone, first time using Phinx and I thought I would add my 2 cents as I really think that names defined in the config file are really misleading for anyone getting started. I would catch the opportunity to clearify a few things for myself and (eventually) open a PR with improved docs.

So, here is a typical config file:

paths:
    migrations: %%PHINX_CONFIG_DIR%%/db/migrations
    seeds: %%PHINX_CONFIG_DIR%%/db/seeds

environments:
    default_migration_table: phinxlog
    default_database: opengrid
    development:
        adapter: pgsql
        host: localhost
        name: opengrid
        user: xxx
        pass: xxx
        port: 5432
        charset: utf8
        schema: main
  1. What is default_database? how does it relate to development.name?
  2. Within envs why is the actual default db called name? I mean, name of what? I honestly think that db_name or more simply db would be a better choice.
  3. Is there a place where I can find all the possible params that I can use in a phinx.yml file? I've been spending quite some time on the docs but couldn't find anything. I just guessed I could use schema property to not use the public schema in pgsql.

Thanks a lot and keep up with the great work :)

jnardiello commented 8 years ago

I might have had just now an epiphany. Is by any chance default_database actually an environment name that would be executed when no -e parameter is supplied by console?

rquadling commented 8 years ago

Confused ... yeah ... you and me both. When I first started, your post is almost word for word the same as mine.

Without a doubt default_database should be default_environment, but that'll probably have to wait for a while - can't change that in a minor or patch release.

And yes, default_database is the default environment to be used when there is no --environment parameter.

jnardiello commented 8 years ago

Thanks @rquadling for the feedback. I think there are two actions we can do immediately to "mitigate" this naming issue:

Also, any idea about the last question? Is there a documented list of available properties for postgresql? I mean, schema is working but I totally tried it randomly. I will try to dive deep into the source code and see if I can find anything.

I also just noticed a bit of code from the PostgresqlAdapter that smells a lot like forcing a column id whenever it is not defined explicitly in the createTable - this is a pretty strong assumption. In my schema I don't have an id column for every table as my primary key is simply another column (with just a different name). But yeah, this is another issue which I will try to check later.

Merry Christmas!

rquadling commented 8 years ago

I think the source is the only place at the moment. Ideally, each setting needs to be documented along with what adapters it is available in.

wandersonwhcr commented 8 years ago

IMHO, in the next version of phinx, if the configuration file has default_database, show a warning in the output with something like "deprecated option, change to default_environment... using default_database as default_environment".

wandersonwhcr commented 8 years ago

... and there's some outputs like

$output->writeln('<info>using database</info> ' . $envOptions['name']);

... that must be changed to using environment.

wandersonwhcr commented 8 years ago

ignore my last comment, it's right to output using database because there is an using enviroment.

WillGibson commented 8 years ago

I got confused by the default_database name too initially. For my two penneth, I think the name could just be default. Then the config might be like this...

environments:
    default_migration_table: phinxlog
    default: vagrant

    vagrant:
        ...

    production:
        ...

I like Phinx though, so I'd be up for putting some lunch breaks into helping on this issue.

rquadling commented 8 years ago

I think changing this would have to be a V1 change. And things don't move too quick here.

WillGibson commented 8 years ago

Does it have to wait til then? Can we do it sooner as long as we don't break backwards compatibility?

rquadling commented 8 years ago

Not my call. But a BC is going to have to sniff different values and deal with conflicts where someone may already have added something to the config.

dereuromark commented 6 years ago

Are you able to provide a patch with your suggested changes as PR?