search

moneyphp / money

PHP implementation of Fowler's Money pattern.
http://moneyphp.org
MIT License
4.57k stars 440 forks source link

Delete composer.lock #722

Closed rogervila closed 1 year ago

rogervila commented 1 year ago

Hello,

Packages usually do not contain a lock file since it can cause unexpected collisions with other packages installed on a project.

Also, it makes it more difficult to support multiple PHP versions since it ties dependencies to specific versions, which might not support newer PHP versions.

rogervila commented 1 year ago

@frederikbosch this PR would solve the issue on #715

rvanlaak commented 1 year ago

Worth knowing is that composer solely uses the root project's lock file, and does not consider lock file of dependencies: https://getcomposer.org/doc/02-libraries.md#lock-file

But yes, given that libraries typically get tested against multiple PHP versions, having a lock file on libs does not contribute much. Having a "test lowest dependencies" test run is more valuable.

sagikazarmark commented 1 year ago

BTW this has been discussed here: #671

I don't really care, but going back and forth isn't that great IMO.

rvanlaak commented 1 year ago

About the related commit message:

PHPCS, PHPStan, Psalm and similar tools are too unstable to lock at minor/major release, therefore we need a composer.lock to keep things frozen during builds.

The CI jobs indeed do run via the dependency versions as pinned down in the lock files, except for the "test lowest" CI job. The mentioned tools do seem much more stable nowadays though.

frederikbosch commented 1 year ago

Sorry guys, I just accepted the change. Maybe I should have left it in. Maybe I should have opened discussion. Don't know. I think it is still rather unusual to commit the composer.lock. It was not the first time a PR came with a changed lock file.

Also considering that amount of PRs is not that much - this library is not in phase of creating features - that chances are when a PR is there, it is when a major dependency has changed - with PHP version being the most likely one. This most probably will lead to updating dev dependencies. This was the case first with the 4.1.0 release, required updating PHP spec and then Psalm. With PHP 8.1 support we were also waiting for a PHP spec update.

So I decided remove composer.lock but tighten Psalm to ~5.3.0, which I think should lead to stable builds too. While going and forth is not that great - agreed - I suggest to see what happens.