search

cloudinary / cloudinary_php

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

[2.7.1] Why separate package and failure on composer-require-checker #359

Closed ruudk closed 2 years ago

ruudk commented 2 years ago

Just noticed on the Dependabot bumpi from 2.7.0 to 2.7.1 that a new package was introduced.

First question: When creating a new package, why o why are you doing that for unsupported PHP versions. It supports PHP 5.6 which was EOL 3 years, 4 months ago 🤷

Also, it doesn't follow PHP best practices using PSR-4 namespaces but uses a class map.

This is also why composer-require-checker GitHub actions fails now on that Dependabot bump PR:

Screenshot 2022-05-30 at 09 04 21@2x
const-cloudinary commented 2 years ago

@ruudk , thank you for your feedback!

We have extracted transformations to a separate package since we will release a few more packages that will depend on it soon.

Regarding PHP version... Yeah, that is a good point. Since this is an SDK and many products/integrations/frameworks use it, we don't want to limit them to the old versions of the SDK, or restrict them, and force them to bump their versions, unless it's absolutely nessesary. If you check packagist, there are many libraries who still have minimum PHP version 5.6.

And it's not that we are in desperate need of the latest PHP features. We will do it at some point for sure. And the code perfectly works with all the versions, including 8.1.

PSR-4. Yeah, we have two main goals here: Provide discoverability for users, while keeping the code base well structured.

Not all of our users use smart IDEs, some use simpler editors, that are not always smart enough to automatically import the required classes. Searching for Resize or Overlay, if taking into consideration the actual file path can be pretty challenging and not very intuitive. That's why we put all of the Actions under Cloudinary\Transformation namespace.

From the other side, if we flatten the transformations folder, it would be a disaster to maintain it.

composer-require-checker - to fix it, you'll probably need to add transformations package explicitely to your project. I completely understand the goal of the tool. From our side we have limited the major version dependency, so you won't have a surprise one day, when we have some breaking changes in the future.

Hope it answers your questions.