search

cloudinary / cloudinary_php

PHP extension for Cloudinary
https://cloudinary.com/documentation/php_integration
MIT License
389 stars 151 forks source link

Allow psr/log v2 and v3 #333

Closed IonBazan closed 2 years ago

IonBazan commented 3 years ago

Brief Summary of Changes

This change allows installing cloudinary/cloudinary_php alongside with psr/log v2 and v3. monolog/monolog has been limited to ^1|^2 as it's not a good practice to use * wildcard.

What does this PR address?

Are tests included?

Reviewer, please note:

There are no BC breaks as we don't rely strongly on psr/log return types.

Checklist:

ruudk commented 2 years ago

@const-cloudinary Can this be merged please? 🙏

const-cloudinary commented 2 years ago

@IonBazan , @ruudk We have this issue: #312 and currently we do not support monolog v2 and v3, since this library is compatible with PHP 5.6

We'll fix that in the next major version.

ruudk commented 2 years ago

And when will that be released?

ruudk commented 2 years ago

I don't see the Travis tests working on this PR. Can those be enabled so that we can see if there is actually an issue?

Because I think Composer automatically selects the correct version.

And also, why would this bundle still support PHP 5.6? It's end of life since 2019!

ruudk commented 2 years ago

After thinking about it more, I think #312 is caused by the wildcard for monolog. That basically says "do whatever you want".

The fix in this PR is therefore good and safe.

const-cloudinary commented 2 years ago

@ruudk , the issue is not with the dependencies, but implementation of LoggerInterface: https://github.com/cloudinary/cloudinary_php/blob/e588606b24f0feda3c1962f270eda698f4b730fd/src/Log/LoggerDecorator.php#L22

Since psr/log interfaces between version 1 and 2 are not that compatible, we can't support both.

We are working on dropping support of old PHP versions because it becomes impossible to maintain it this way.

It might take some time, it will require other updates in the code to make it fully compatible with the latest versions of PHP.

ruudk commented 2 years ago

What about mitigating the LoggerDecorator issue like this: https://github.com/symfony/phpunit-bridge/blob/15cab721487b7bf43ad545a1e7d0095782e26f8c/SetUpTearDownTrait.php

I hope that dropping PHP 5.6 won't cause a big refactor project and therefore delays this 😊

const-cloudinary commented 2 years ago

@IonBazan, thank you for contribution! @ruudk, thank you for providing suggested solution for the issue!

The issue is resolved in #336

Unfortunately we are in a code freeze for Christmas holidays. The next version of the SDK will be released in January.

ruudk commented 2 years ago

Amazing! Thank you and happy holidays

ruudk commented 2 years ago

Can we new release be tagged please? 🙏

const-cloudinary commented 2 years ago

@ruudk , sorry for a delay, the new version is released:

https://github.com/cloudinary/cloudinary_php/releases/tag/2.6.0