About libraries in OpenMage. Direct usage or Fork?

[ADDISON74]

As you may know we have in plan to migrate all 3rd party libraries to composer dependencies. There is an open PR for a part of them here #2411.

Now please read this post created in a module that we use MyIntervals/emogrifier#1164 (comment)

The author then comes and offers explanations, presenting his point of view why he proposed to drop support for PHP 7.3.

We recently took a step by establishing as the minimum PHP requirement in OpenMage version 7.3, even is EOL we find it in operating systems such as Debian 10 and which will be maintained until June 2024.

The discussion I want to propose is related to these libraries that we want Composer to manage. Until now, OpenMage had all the files in its own repository, but with the modifications we want to make we become dependent on the decisions of those who maintain these projects.

I think we should consider having our own forks of these projects right here in OpenMage, along with magento-lts. In this way, we can avoid being taken by surprise by radical changes where we do not have immediate solutions.

[Flyingmana]

I would say, avoid having a fork of them as long as possible.
And most surprises should be avoidable by setting version constraints which dont allow new major versions to be installed automatically.

[fballiano]

actually I discovered something just now that makes it not necessary to fork the net_idna2 repo, it's possible to link a specific commit ID in the composer.json, I just did it here: https://github.com/OpenMage/magento-lts/pull/2411/files#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34 so at least in this case I feel comfortable releasing this specific version of that library :-)

[fballiano]

auch sorry guys, my local composer was working fine but the one in the workflows tells that linking to a commit ID is a bad practice:

although I don't see why it should it a bad practise

[sreichel]

It seems to be a warning, but the error comes from outdated lock-file.

Edit: :(

--strict: Return a non-zero exit code for warnings as well as errors

[fballiano]

yep, it's the strict validation

[sreichel]

I'd use "dev-main", instead of forking just to have a tagged release. This PHP/PEAR lib has no php requirement and won't see any BC breaking changes ... i'm pretty sure.