foundation / proton

CLI Tool for compiling web pages and email
MIT License
21 stars 3 forks source link

Composer updates #14

Closed phpfui closed 2 years ago

phpfui commented 2 years ago

Joe,

I will probably never use this library, as I use PHPFUI/InstaDoc to output HTML, but thought I would help you on some minor composer issues.

First, you can't use semantic versioning with PHP versions. Common mistake. I wrote a handy version checker here. I never fail to not get it right the first time. This helps. I also added 8.1 support, as that is the current version. You should probably drop 7.3 and 7.4 support, as it is super easy to run specific versions of PHP in a non prod env, so no reason not to leverage the better versions. 8.1 is not quite ready for prime time. I submitted, and they already fixed, an enum bug, so waiting for 8.1.2 before I do more enum work.

Also removed composer.lock, which is generally only used for releases into production, not for libraries, although you could make an argument to use it here, but thinking letting users update things on their end is best practice, who knows what will be fixed next.

Also added roave/security-advisories that will check for security issues automatically.

DanielRuf commented 2 years ago

@phpfui SemVer works but it should be probably ||.

See also https://github.com/shopware/core/blob/trunk/composer.json#L33

phpfui commented 2 years ago

I as actually confused by all the versioning tricks Composer supports, so I ended up writing my own checker (could not find an existing one anywhere). You can see it in action here: (http://instadoc/Examples/ComposerVersion.php). You can also view the source code (based on the Foundation documentation idea of viewable source!) if you want to check out the logic behind it. I think it is correct from what I can tell.

Anyway, the basic issue that many people don't understand is PHP does not use the standard semantic versioning that Composer uses. For example, normally a package major version will 6, with minor versions like 6.1, 6.2, etc. The ^ implies take the highest minor version update available, since 6.1 should be compatible with 6.0, but with some new features.

This is not how PHP is versioned. PHP 8.1 breaks some things in PHP 8.0 for example (or maybe just issues warnings, whatever). The point is you probably don't want the package as it currently exists to be able to run on PHP 8.9, which is what the current Composer files allows, as you have no idea of what PHP 8.9 will even look like.

So my preferred version specification is ">=7.4 <8.2" which clearly says (with the implicit &) that you require 7.4, 8.0, or 8.1, but nothing higher or lower.

As I said, common mistake. I see it all the time.

DanielRuf commented 2 years ago

Partically true. It's true thst warninhs and deprecated features may be errors and removed in the next 8.x release, that is normal. Most stable features are not affected by this in my experience.

Besides this ~ is a better approach. See for example https://github.com/magento/magento2/blob/2.4-develop/composer.json#L14

phpfui commented 2 years ago

That does not allow for 8.0. Again, this stuff is not obvious. Which is why I like my solution.

DanielRuf commented 2 years ago

Sure. That's why we use also ~8.0 in projects. Just saying that there is a valid SemVer selector.

But in most cases projects should just set the start range like >= 7.4. Normally you don't set the upper part. See for example phpunit (18 years old project): https://github.com/sebastianbergmann/phpunit/blob/8.5/composer.json#L24 ;-)

joeworkman commented 2 years ago

I finally merged this all in. I had made some other changes that caused conflicts so I will close this. The changes are in v0.6.1. Thanks @phpfui