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

Added support for foreign key and unique constraints #1

Merged
merged 1 commit into from
Jan 4, 2016

Conversation

LenzLueers
Copy link
Contributor

No description provided.

gonzalo123 added a commit that referenced this pull request Jan 4, 2016
Added support for foreign key and unique constraints
@gonzalo123 gonzalo123 merged commit ec80678 into gonzalo123:master Jan 4, 2016
@gonzalo123
Copy link
Owner

Hey. Thanks. I must admit this project is quite abandoned. 5 yo without any change, without any test coverage I hope that it still works :)

@LenzLueers
Copy link
Contributor Author

Hey Gonzalo,

Thanks form merging. Actually I tested several “state of the art” database migration tools for PHP (phinx, ruckusing/migrations, CakeDC/Migrations) and decided, yours works the best for my purpose ;-)
Yeah, there is no test coverage and there are still some issues (new creation of tables does still not work for all constraints, so you need to run it twice initially), but it seems stable enough to me to set it on my current productive project.

I think during the next days I’m still going to commit some optimizations. I’d also like to publish it as a composer package. Do you have any reservations about that or do you want to do that?

Cheers,
Lenz

@gonzalo123
Copy link
Owner

I'll create composer package today or maybe tomorrow. Nowadays I always add composer and at least a few integration test to projects. But 5 years not ago (good excuse, isn't it? :)

Here maybe I need to create also too a travis.ci integration with a fake database for testing (as far as I remember Travis allows to us postgresql). I will try not to touch library to allow to merge your commits without problems.

Also maybe you can have a look to a friend's library https://github.com/ojoven/mirendb. It's something similar but with a different scope

@gonzalo123
Copy link
Owner

I've created a new branch of the project. With psr-4 support. and composer friendly.
It works for you?

TODO: A lot of things. Mainly:

  • cli sucks. OK. It haven't got any dependency, but there's no award for it. For example to use symfony Console Component should be great here.
  • Tests, tests and more tests. Those kind of libraries cannot be done without test coverage. IMHO it's not difficult to add tests here from scratch (creating database and tables within each iteration) and allowing travis to execute those tests (I'm working on it, indeed)
  • tabs and spaces together tabs vs spaces

@LenzLueers
Copy link
Contributor Author

Hey Gonzalo,

Thats great news! I'll test the composer integration tomorrow.

And for the cli: yeah, its not super beautyful, but most people (at least me) are going to set it up once in their CI tools. I'm going to write a rocketeer task for that.

For the code conventions: I used psr2, but psr4 is fine with me. So let's get rid of the tabs ;-)

I have a few new changes (linking of auto increment sequences for primary keys, complete constraint creation on table creation) almost done. I'm probably going to commit them tomorrow.

@gonzalo123
Copy link
Owner

New branch is called v2. It also has a simple set of tests. The library code is unchanged (with the same bugs). There's only a small change in folder structure (due to PSR-4). If it's ok for you I can merge branch

@LenzLueers
Copy link
Contributor Author

Thats perfectly fine with me. I couldn't take the time today to test my newest changes, so I'll probably commit them next week.
Thanks for the cleanup!

@gonzalo123
Copy link
Owner

Ok I will merge the branch to master

@LenzLueers
Copy link
Contributor Author

I pushed some new input. Unfortunately I couldn't come up with a perfect solution for foreign key creation on initial structure creation, since related tables may not yet exist. I introduced a new cli parameter -k to skip foreign key creation manually, but this is far from a great solution. Another option would be to run process the foreign keys after all other scipts.

Another question: Is there a reason, why you removed the executable php script "pgdbsync" / "cli"?

@gonzalo123
Copy link
Owner

I've moved pgdbsync to bin dir and my default .gitignore excludes this folder (fixed)

-k parameter sounds fuzzy maybe is better not to create fk (show a warning) first time and do it in the second time. Now is more easy to test the problem. Unit test with database normally are nightmare but the tests I've create are very simple and allow detect bugs. Can you create a failing test to show this issue.

In fact the library is far from being perfect. I've realised various problems that slow down the development process:

  • It's strongly coupled to PDO object. I've had problems when I tried to integrate with travis. DbConn Object should accept an injected PDO object rather than create a new one
  • The library should be splitted in two components. One to detect differences and another one to parse this differences into a sql dialect.
  • At least Db class needs  a big refactor. There're a lot of warnings and unused parameters everywhere.
  • the lack of unit tests is a big problem. I can see various errors at glance, but it's difficult to ensure the fixtures don't break the working parts.

I'll try to find time to create more tests, anyway

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.

None yet

2 participants