clerkio / clerk-magento2

Magento 2 extension for Clerk.io
MIT License
9 stars 29 forks source link

Coding standard dev requirement causing dependency issue #90

Closed jupiterhs closed 1 year ago

jupiterhs commented 1 year ago

Magento version : 2.4.3-p1 Clerk version : 2.7.2 (Upgraded from 2.7.1)

In Magentos composer.json there is a dev requirement defined as "magento/magento-coding-standard": "*" so that a compatible version of the coding standard module gets installed. In version 2.7.2 of your module, you have restricted this by adding a dev requirement for magento/magento-coding-standard:^27.0.

Version 27 of the coding standard module requires at least PHP 8.1, so it causes a dependency issue, thus not letting me install your module, when I try on Magento 2.4.3-p1 which is running on PHP 7.4.

By adding this dev requirement I think you have unintentionally removed compatibility for previous versions of Magento.

Also, I know that I can use a workaround by installing the module using the --update-no-dev option of composer require, but this causes a problem because I need Magento coding standard to be installed and this option removes all the require-dev modules.

stubbedev commented 1 year ago

I have reverted the require-dev change in the master, at least until we officially deprecate php 7 from our support. Thank you for posting.

jupiterhs commented 1 year ago

Hi @FoulBachelor I took a look at your commit
With this change your module will not install on Magento 2.4.3-p1 as you have forced coding standard version 27 as a requirement. You would need to remove "require": { "magento/magento-coding-standard": "^27.0" } and let magento handle the right version that should be installed.

Is there any reason you want to add this as a requirement?

stubbedev commented 1 year ago

I have reverted the require from the master for now, as it seems to have caused a few issues.

https://github.com/clerkio/clerk-magento2/commit/5e5a23b4e9a58201578136bd5e2eef198b9f6e06

For you current setup, you can simply delete the requirement from the package release or install from current master.

The reason we want to add this requirement, is due to some issue we see with certain clients keeping their PHP versions far from current, as well as some compliance for moving the extension to the marketplace.

@jupiterhs In terms of Salecto's specific offering of Magento 2, it would be nice to have a full spec sheet for the implementation you offer clients forwarded to our partnerships department.

Eg. M2 version + patch, PHP version, DB version, as well as info on the Apache / Nginx setup type in case you utilize reverse proxies. Otherwise it is difficult to handle your specific needs with greater care than the average anonymous user of our plugin.

mortenbirkelund commented 1 year ago

I dont see how this is related to DB version, apache, Nginx or even directly PHP?

If you don't support Magento 2.4.3 anymore, then do it the proper way; add a dependency for the magento framework version you support and be done with it. It seems very strange to do it indirectly by setting a dev requirement for the magento coding standards, and even more strange the way you try to solve it?

I will have a discussion with your partner manager.

stubbedev commented 1 year ago

Thank you, I look forward to receiving details. :)