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

CakePHP 4.x compatibility #19

Merged
merged 8 commits into from
Feb 17, 2020
Merged

CakePHP 4.x compatibility #19

merged 8 commits into from
Feb 17, 2020

Conversation

swiffer
Copy link
Contributor

@swiffer swiffer commented Dec 28, 2019

  • code style
  • type hints

@ADmad ADmad changed the base branch from master to cake-4.x December 28, 2019 12:33
@swiffer
Copy link
Contributor Author

swiffer commented Dec 28, 2019

@ADmad I've added compatibility for Postgres as well (https://stackoverflow.com/a/13948327).

Can you take a look please?

@ADmad
Copy link
Member

ADmad commented Dec 28, 2019

I don't have enough knowledge regarding Postgres to be able to review that. Hopefully someone else can.

@swiffer
Copy link
Contributor Author

swiffer commented Dec 28, 2019

I've tested the Statement locally, worked fine here

revert add support for postgres. this does not work when a table changes that has foreign key constraints.
@swiffer
Copy link
Contributor Author

swiffer commented Dec 29, 2019

@ADmad I had to revert the changes for postgres because it does not work when foreign key constraints exists. this is CakePHP 4.0 compatibility only now.

@swiffer swiffer requested a review from jippi December 29, 2019 13:11
@swiffer
Copy link
Contributor Author

swiffer commented Jan 2, 2020

@jippi can you take a look again so we can get this merged ?

@dakota
Copy link
Member

dakota commented Feb 13, 2020

What is still needed for this?

Copy link
Member

@ADmad ADmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dakota dakota merged commit 0985c03 into FriendsOfCake:cake-4.x Feb 17, 2020
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.

4 participants