Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore composer.lock #251

Closed

Conversation

kohlerdominik
Copy link
Contributor

composer.lock should not be included in packages, only in projects. See for example the symfony package we use ourselves:

It was an issue for me, because in the GithubAction I was looking for the actual versions used, but they are not listed if they do not differ from composer.lock. This is why I suggest this fix, which resolves a bad practice AND shows the actual dependecy versions used in the GithubAction for easy lookup.

@kohlerdominik
Copy link
Contributor Author

So in the now failing pipelines we can actually see the issue I was refering to. In prefer-stable pipelines the tests are failing, and we can now see all the package versions used:
grafik

→ The Pipeline failing is not related to this change, but to one of the dependencies, I assume. With the latest discussions, I assume it's endroid/qr-code?

@sprain
Copy link
Owner

sprain commented May 15, 2024

Thanks!
There is overlap with #252 and therefore handling this right there, as well.

@sprain sprain closed this May 15, 2024
@PowerKiKi
Copy link

I think this PR should be reverted. First, committing composer.lock is never a bad practice, being a lib or a project. A lib has dev dependencies that must be predictable for development cycle.

Without composer.lock, you have no idea which version de deps you will get, locally and on GitHub Actions, and it might or might not break the build on code that you did not even touch, locally or on GitHub Actions.

Without composer.lock, you have to fallback on ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}. That means that GitHub Actions will not refresh its dependencies static code analysis, if composer.json is changed, for any reasons. That is a very unpredictable behavior that will bite you at a later time.

composer.lock should always be committed to have a guaranteed stable dev environnement. Then, in addition to that, if you are willing to also test oldest and newest dependencies that may break at any moment, be my guest, but don't remove the guarantee to have reproducible, stable dev environment both locally and in CI.

@kohlerdominik
Copy link
Contributor Author

kohlerdominik commented Jun 7, 2024

Without composer.lock, you have no idea which version de deps you will get, locally and on GitHub Actions, and it might or might not break the build on code that you did not even touch, locally or on GitHub Actions.

That's not true. I think you're mixing up composer.lock and composer.json, from where the later is very much present. In composer.json, the compatible versions are defined.

composer.lock should always be committed to have a guaranteed stable dev environnement. Then, in addition to that, if you are willing to also test oldest and newest dependencies that may break at any moment, be my guest, but don't remove the guarantee to have reproducible, stable dev environment both locally and in CI.

Again stable dev environment is defined in composer.json. All compatible versions MUST be defined there and it MUST not matter what version of the defined is pulled. It must be compatible.

Without composer.lock, you have no idea which version de deps you will get, locally and on GitHub Actions, and it might or might not break the build on code that you did not even touch, locally or on GitHub Actions.

This is very much intended. Because build SHOULD break on package level rather than as project level. Without updating packages in the pipeline, it's not garantued to be compatible with new minor version. If this happens, it means that composer.json is incorrect. Could also be a dependency issue because a dependecy change defined as non-breaking is actually a breaking change. No matter what, this must be catched as early as possible.

@PowerKiKi
Copy link

I define "stable dev environment" as "identical patch level version of all packages". composer.json typically only gives you a guarantee at the minor level (unless you pin exact version in your composer.json, which is thankfully super-rare in PHP ecosystem). Only composer.lock can give you the guarantee to have exactly the same dependencies, at a patch level.

And the unfortunate reality is that bugs and/or minor change of behavior are sometimes introduced in patch level. PHP-CS-Fixer might slightly change their formatting, or PHPUnit might starts to deprecate things. You probably don't want to deal with that when you're already busy trying to solve something else entirely.

this must be caught as early as possible

Yes, I agree, but that should be in addition to a stable CI step, not instead of. You wouldn't test only with the nightly build of PHP. You test with a stable, well-defined version of PHP, and then, in addition, you try your luck with the nightly build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants