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.

Sign up for GitHub

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

Jump to bottom

feat: allow doctrine/dbal v4 #247

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: allow doctrine/dbal v4 #247

wants to merge 1 commit into from

Conversation

kochen
Copy link

@kochen kochen commented Feb 7, 2024 •
edited

Description

Support doctrine/dbal:^4.0

Closes: #246

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • I have added tests to cover my changes.

@kochen kochen requested a review from ramsey as a code owner February 7, 2024 09:17
This was referenced Feb 10, 2024
Closed
Closed
@kochen
Copy link
Author

kochen commented Mar 8, 2024

@ramsey could you please take a look?

greg0ire
greg0ire reviewed Mar 12, 2024
View reviewed changes
composer.json Outdated Show resolved Hide resolved
@greg0ire
Copy link

@kochen I've created #253 to show how it could be possible to avoid forcing users to upgrade both packages at once. Feel free to incorporate the changes into your PR.

@greg0ire greg0ire mentioned this pull request Mar 12, 2024
Closed
@kochen
Copy link
Author

kochen commented Mar 13, 2024

Thanks @greg0ire - on it...

@kochen
Copy link
Author

kochen commented Mar 13, 2024

@greg0ire @ramsey now supporting both dbal v3 (& v2.8) as well as v4!

@kochen kochen changed the title feat: allow doctrine/dbal v4 and remove older versions feat: allow doctrine/dbal v4 Mar 13, 2024
@kochen kochen requested a review from greg0ire March 15, 2024 09:55
greg0ire
greg0ire approved these changes Mar 15, 2024
View reviewed changes
Copy link

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Looks great. I think some parts of your PR have nothing to do directly with doctrine/dbal 4 compat and you could split them out in a separate commit or even pull request for clarity.

@kochen
Copy link
Author

kochen commented Mar 18, 2024

Hi @greg0ire,
I tried reverting each of the changes I made and they all seem to be required for dbal v4.

Do you have one specific suspect you could point out?

greg0ire
greg0ire reviewed Mar 18, 2024
View reviewed changes
composer.json Outdated Show resolved Hide resolved
@tasselchof
Copy link

What need to be done here to get this pull request approved @greg0ire @ramsey?

@greg0ire
Copy link

I don't know, I'm not a maintainer of this repository

@tasselchof
Copy link

I don't know, I'm not a maintainer of this repository

Oh, I didn't realize you were just helping with this commit. Let's wait for the maintainer.

@greg0ire
Copy link

I'm a Doctrine maintainer, and I want DBAL 4 to get more adoption, hence my review :)

@tasselchof tasselchof mentioned this pull request Apr 17, 2024
Open
@tasselchof

This comment was marked as off-topic.

Sign in to view
@greg0ire

This comment was marked as off-topic.

Sign in to view
@tasselchof

This comment was marked as off-topic.

Sign in to view
@greg0ire

This comment was marked as off-topic.

Sign in to view
@tasselchof

This comment was marked as off-topic.

Sign in to view
@greg0ire

This comment was marked as off-topic.

Sign in to view
@tasselchof

This comment was marked as off-topic.

Sign in to view
@greg0ire

This comment was marked as off-topic.

Sign in to view
@tasselchof

This comment was marked as off-topic.

Sign in to view
@greg0ire

This comment was marked as off-topic.

Sign in to view
@tasselchof

This comment was marked as off-topic.

Sign in to view
@tasselchof

This comment was marked as outdated.

Sign in to view
@greg0ire

This comment was marked as off-topic.

Sign in to view
@tasselchof

This comment was marked as off-topic.

Sign in to view
@greg0ire

This comment was marked as off-topic.

Sign in to view
@ramsey
Copy link
Owner

ramsey commented Apr 27, 2024

This looks good, but it's failing the static analysis checks, and I don't have time to investigate. Can you look into it?

@kochen
Copy link
Author

kochen commented Apr 29, 2024

This looks good, but it's failing the static analysis checks, and I don't have time to investigate. Can you look into it?

@ramsey all the issues are related to the unsupported PHP 7.4
Is there any plan to drop support for it? doctrine/dbal:4 expects PHP > 8.1 anyway...

@kochen
Copy link
Author

kochen commented Apr 29, 2024

@ramsey could you approve the workflow again?

@kingjia90 kingjia90 mentioned this pull request Apr 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire greg0ire approved these changes

@ramsey ramsey ramsey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Doctrine/dbal v4 is out!
4 participants
@kochen @greg0ire @tasselchof @ramsey