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

MySQL "ADD COLUMN ... AFTER ..." support #216

Merged
merged 7 commits into from Feb 23, 2017

Conversation

Projects
None yet
4 participants
@nanawel
Contributor

nanawel commented Feb 10, 2017

ZF1 supports MySQL syntax allowing to add a column AFTER another one in an ALTER TABLE statement. ZF2 does not. This PR is intended to fix that.

@nanawel nanawel changed the base branch from master to develop Feb 10, 2017

@alextech

This comment has been minimized.

Show comment
Hide comment
@alextech

alextech Feb 10, 2017

Contributor

I want to say "needs a test" but looking at the decorator, there are no existing tests for it where you could have added it. Yet it must be done :) Can you please create an AlterTableDecoratorTest in the same place and same way CreateTableDecoratorTest and add your option in there? (I could assist if unsure how)

Contributor

alextech commented Feb 10, 2017

I want to say "needs a test" but looking at the decorator, there are no existing tests for it where you could have added it. Yet it must be done :) Can you please create an AlterTableDecoratorTest in the same place and same way CreateTableDecoratorTest and add your option in there? (I could assist if unsure how)

@nanawel

This comment has been minimized.

Show comment
Hide comment
@nanawel

nanawel Feb 10, 2017

Contributor

Sure, I'll add it to the PR.

Edit: seems like I included more commits than intended, I'll check that first.

Contributor

nanawel commented Feb 10, 2017

Sure, I'll add it to the PR.

Edit: seems like I included more commits than intended, I'll check that first.

@alextech

This comment has been minimized.

Show comment
Hide comment
@alextech

alextech Feb 10, 2017

Contributor

Thank you :)

Edit: I am working under assumption that even though overly database specific modifications are usually rejected, since there is already a MySqlAlterTableDecorator, adding to it should not be a problem.

Contributor

alextech commented Feb 10, 2017

Thank you :)

Edit: I am working under assumption that even though overly database specific modifications are usually rejected, since there is already a MySqlAlterTableDecorator, adding to it should not be a problem.

@nanawel

This comment has been minimized.

Show comment
Hide comment
@nanawel

nanawel Feb 10, 2017

Contributor

Yes, this is my assumption too.

Contributor

nanawel commented Feb 10, 2017

Yes, this is my assumption too.

@nanawel

This comment has been minimized.

Show comment
Hide comment
@nanawel

nanawel Feb 10, 2017

Contributor

New test class included, no error detected on PHPUnit on my instance.

Contributor

nanawel commented Feb 10, 2017

New test class included, no error detected on PHPUnit on my instance.

@ezimuel ezimuel self-assigned this Feb 23, 2017

@ezimuel ezimuel added the enhancement label Feb 23, 2017

@ezimuel ezimuel merged commit db47b5a into zendframework:develop Feb 23, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.008%) to 46.044%
Details
@ezimuel

This comment has been minimized.

Show comment
Hide comment
@ezimuel

ezimuel Feb 23, 2017

Member

@nanawel thanks for your contribution!

Member

ezimuel commented Feb 23, 2017

@nanawel thanks for your contribution!

@nanawel nanawel deleted the nanawel:mysql-add-column-after-support branch Dec 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment