search

sonata-project / EntityAuditBundle

Audit for Doctrine Entities
https://sonata-project.org
MIT License
633 stars 261 forks source link

Allow Doctrine DBAL 3 #444

Closed jordisala1991 closed 3 years ago

jordisala1991 commented 3 years ago

Subject

I am targeting this branch, because this is BC.

Changelog

### Added
- Added support for Doctrine DBAL 3.
jordisala1991 commented 3 years ago

Not sure with any of this changes, just tried to follow deprecation messages. This bundle blocks the doctrine orm admin bundle. @phansys or @VincentLanglet do you know why test fail?

simonberger commented 3 years ago

@jordisala1991 The changes are looking good to me. Didn't check if they are complete.

To the tests failures it is probably this: https://github.com/doctrine/dbal/blob/2.13.x/lib/Doctrine/DBAL/Schema/Column.php#L78-L89

Which means already a problem in the current releases and another deprecation in dbal 2.X which has to be solved.

simonberger commented 3 years ago

Just a side note: I would prefer tests to fail when hitting a deprecation or at least displaying it. I saw no such output in the previous runs of PHPUnit.

VincentLanglet commented 3 years ago

https://github.com/sonata-project/EntityAuditBundle/pull/446 might help @jordisala1991

jordisala1991 commented 3 years ago

I think it would be nice to get your review too @simonberger

jordisala1991 commented 3 years ago

Changes in this PR look good to me as well.

Approved with minor comment:

The use of the SQLResultCasing trait is not optimal. Changes or a removal could happen in every version.

Considering the minimal code it provides the trait probably should be copied into this library. Didn't mention it in the other PR because I thought the solution here will be different.

Do you want to provide a PR with that changes? I think we can add that trait to our library too