imgix-php icon indicating copy to clipboard operation
imgix-php copied to clipboard

build: don't commit the `composer.lock` file

Open adevade opened this issue 2 years ago • 3 comments

Right now, the composer.lock file is committed to the repo. This is usually not done for packages, because it locks it to the PHP version of the person generating the lock file. (See bottom of file)

I think the removal of this file could solve the problem where you have to run composer update in CI as well. Since if the lock file is missing, a new one will be generated upon running composer install. This will happen for each version of PHP in the matrix, letting Composer install the latest packages compatible with the current PHP version.

The lock file is not used when installing this as a package in a project in any case.

Hope you understand my ramblings! 😅

Popular packages without composer.lock files:

adevade avatar Feb 01 '23 12:02 adevade

Hey @adevade, thanks for opening the issue and the PR.

I can see why committing the lock file is inconvenient.

I would like to keep dependencies pinned as that keeps future maintainers and CI from inadvertently updating a dependency, making the build output slightly different from the distributable. But to your point, the Composer docs explicitly state committing the lock file is optional and probably not needed.

For your library you may commit the composer.lock file if you want to. This can help your team to always test against the same dependency versions. However, this lock file will not have any effect on other projects that depend on it. It only has an effect on the main project.

In your opinion, do you see it as an insignificant risk to potentially have dependency versions vary between maintainers so long as CI is always pulling in the latest dependency?

luqven avatar Feb 01 '23 17:02 luqven

From what I've seen in the PHP community, it's most common not to commit the lock file. As seen in my examples above.

Besides, this library has very few dependencies compared to most. Both PHPUnit and the Laravel team are respectable developers, and handle the semantic versioning very well. As long as the version definitions in composer.json is strict enough (and I believe they are), I don't think there will be any problems. 😊

You could configure CI to run twice as well. Once with the latest versions, and once with the lowest supported versions according to composer.json.

--prefer-lowest: Prefer lowest versions of dependencies. Useful for testing minimal versions of requirements[...] Composer flag --prefer-lowest


matrix:
  php: ['8.0', 8.1, 8.2]
  stability: [prefer-lowest, prefer-stable]

From Laravels testing CI

adevade avatar Feb 01 '23 18:02 adevade

I like the CI suggestion a lot, and your explanation regarding relying on composer.json makes sense to me.

luqven avatar Feb 01 '23 22:02 luqven