myparcelnl / prestashop

PrestaShop module to integrate with MyParcel NL and BE
https://developer.myparcel.nl/nl/documentatie/11.prestashop.html
10 stars 5 forks source link

Required PHP version and undefined constant error #215

Closed mariuszsienkiewicz closed 9 months ago

mariuszsienkiewicz commented 1 year ago

Module version

1.7.1

PrestaShop version

1.7.7.1

PHP version

7.2

What went wrong?

There are a couple of issues now. I will try to describe those problems below.

Required PHP version

Please don't specify that this module is compatible with PHP 7.2:

https://github.com/myparcelnl/prestashop/blob/5a1129f8444880b85e06f74587e2e693cb39d055/composer.json#L9-L13

When it's clearly not, your dependencies require at least 7.4.0 (dependencies that require php 7.3+/7.4: laravel/serializable-closure, php-di/invoker, php-di/php-di, psr/container), so it's misleading.

If you really want to support php 7.2.0+ then it would be handy for you to create some Github actions that could run at least some module tests (or just run composer install) using php7.2/7.3/7.4 etc., so it would be much easier to spot those mistakes.

But if you don't want to support versions then at least specify requirements changes in the releases.

INSURANCE_CONFIGURATION_MAX_AMOUNT_EU

I noticed that when trying to upgrade the module from 1.7.1 to 1.8.1 this error pops up:

Undefined class constant 'INSURANCE_CONFIGURATION_MAX_AMOUNT_EU'
[Symfony\Component\Debug\Exception\FatalThrowableError 0]

Reproduction steps

When it comes to INSURANCE_CONFIGURATION_MAX_AMOUNT_EU, I just tried to upgrade the module from 1.7.1 to 1.8.1 via BO.

Relevant log output

No response

Additional context

No response

EdieLemoine commented 9 months ago

Hi @mariuszsienkiewicz, the first beta version of the next version of our module is out now where the bug you mention does not occur anymore.

Please read this issue for more information and how to report bugs in the new version, if you're interested in trying it out.

As for the php version, you're right, thanks for mentioning it. Ironically we've forgotten to update the version in the new beta as well, even though we're aware it requires PHP 7.4. You'll see it updated in the next version we release.