search

extdn / github-actions-m2

137 stars 46 forks source link

Add composer 2 to docker images #67

Closed BorisovskiP closed 2 years ago

jissereitsma commented 2 years ago

Yes but no but yes but no. The current PR only adds a new executable composer to the /usr/local/bin/ without it being used. Also, within these Magento Coding Standards workflows, the only place where composer is used is to install magento/magento-coding-standard which actually has no dependency with composer at all. In other words, I think it would be safe to simply remove all instances of composer 1 with composer 2.

Is there any other thing I'm not seeing here?

sprankhub commented 2 years ago

The most important part here was to add Composer 2 to magento-integration-tests/Dockerfile:7.3. Because otherwise, you could not use Composer 2 in PHP 7.3. And for the integration tests, it makes sense, because you can actually configure the used Composer version.

However, I did not dig into the other containers. If you cannot switch the Composer version there, it indeed does not make much sense. On the other hand, if we still want to support Magento versions, which do not support Composer 2, we should not simply switch to Composer 2 completely.

jissereitsma commented 2 years ago

Yeah, let's fix that issue when somebody asks for it. Composer 1 is deprecated anyway and I'm not sure how much time we should spend on old PHP versions (PHP 7.2 and older), old Magento versions (Magento 2.2 and older) and old composer versions (v1). Let's keep it as it is now, with composer 2 :)

sprankhub commented 2 years ago

Please mind that currently, Composer 2 is available in the containers, but not used by default. So this:

Let's keep it as it is now, with composer 2 :)

Sounds like a wrong assumption :)

jissereitsma commented 2 years ago

O really. I'm a sneaky bastard though: https://github.com/extdn/github-actions-m2/blob/master/magento-coding-standard/Dockerfile:8.1#L3