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

Refacto Query / Statement #118

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

leNEKO
Copy link

@leNEKO leNEKO commented Sep 7, 2023

@ecourtial
Copy link
Collaborator

Hey :D
Thanks for this work!
The issue in the pipeline seems to come from the fact that we are too lax with the PHPUnit version.
I will take some time to update the tests to make them match the requirement of PHPUnit 10.
I had to do it on another project, that's not fun.

@leNEKO
Copy link
Author

leNEKO commented Sep 8, 2023

Hey :D Thanks for this work! The issue in the pipeline seems to come from the fact that we are too lax with the PHPUnit version. I will take some time to update the tests to make them match the requirement of PHPUnit 10. I had to do it on another project, that's not fun.

@ecourtial did fix the various issues with phpstan / phpunit / phpcsfixer and make infection working again in dev env :)

@ecourtial ecourtial self-requested a review September 10, 2023 09:20
ecourtial
ecourtial previously approved these changes Sep 10, 2023
Copy link
Collaborator

@ecourtial ecourtial left a comment

Choose a reason for hiding this comment

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

@leNEKO OK for me. Just need an update in the changelog :)
Huge work !

@ecourtial
Copy link
Collaborator

@leNEKO For the changelog, you don't have to be specific. Just put the overall idea.

@leNEKO
Copy link
Author

leNEKO commented Sep 10, 2023

@leNEKO For the changelog, you don't have to be specific. Just put the overall idea.

Here it is fad65cb?short_path=02aa01b#diff-02aa01b7a281606221d6b6141e30a15d4bb22a980ad403bde95b89aa3588213f let me know if it is enough :)

@leNEKO leNEKO marked this pull request as draft September 11, 2023 01:06
@leNEKO leNEKO marked this pull request as ready for review September 11, 2023 03:14
@ecourtial
Copy link
Collaborator

@leNEKO Allright.
If it is all good for you, I am going to merge and tag a new version.

@leNEKO
Copy link
Author

leNEKO commented Sep 11, 2023

@ecourtial ... i did some additional integration tests with this branch.
` backtics are in fact non standard SQL used by MySQL, and not supported by Postgresql (and probably others).

It need more rework... i set this PR back in draft, and see if i can do something like suggested here #2

@leNEKO leNEKO marked this pull request as draft September 11, 2023 19:21
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.

2 participants