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

Relax the composer pin to make it less confusing #5714

Merged

Conversation

jeffwidman
Copy link
Member

The previous pin technically allowed any version of composer >= 1.10.9, < 2.0... but because it included the full version pin, it was easy to overlook the ^ and think it was pulling in that older version of composer.

Over in the v2/composer.json file, we only pin to `"^2", so I made this pin less explicit for both improved consistency and less confusion.

There is no functional change to composer.lock, so this shouldn't have any impact beyond clearer documentation.

@jeffwidman jeffwidman requested a review from jurre September 13, 2022 21:39
@jeffwidman jeffwidman requested a review from a team as a code owner September 13, 2022 21:39
The previous pin technically allowed any version of composer >= 1.10.9,
< 2.0... but because it included the full version pin, it was easy to
overlook the `^` and think it was pulling in that older version of
composer.

Over in the `v2/composer.json` file, we only pin to `"^2", so I made
this pin less explicit for both improved consistency and less confusion.

There is no functional change to `composer.lock`, so this shouldn't have
any impact beyond clearer documentation.
@jeffwidman jeffwidman force-pushed the make-composer-pin-less-confusing branch from 753c3ef to 5c97c23 Compare September 13, 2022 22:43
@@ -2,7 +2,7 @@
"require": {
"php": "^7.4",
"ext-json": "*",
"composer/composer": "^1.10.9"
"composer/composer": "^1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're going from >=1.10.9, <2.0.0 to >=1.0.0, <2.0.0, the only thing I wonder about is whether we were relying on anything specific about 1.10.9 as the lower bound?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I checked git blame for that when I was researching this... it looked like folks were simply bumping it every so often when they remembered to keep it in sync with the version of Composer in the Dockerfile... nothing special. It just creates confusion though because we've got a very explicit number here, and then another one in composer.lock and then another one in the Dockerfile... and the other two are the ones that actually matter for us, but it's easy for someone less familiar with the composer ecosystem to think that this pin is affecting things. That was effectively me a few hours ago before I read up on this. 😀

I also observed the surprising behavior that historically Dependabot opens version bump PRs in this same composer.json file for other libraries that are pinned to explicit versions like this... so not sure why it didn't for this version? the only difference was that it was in require-dev section and that it used a ~. Given that this ecosystem is mostly EOL'd, I didn't dig into which of the above two it was.

@jeffwidman jeffwidman merged commit 9e15f16 into dependabot:main Sep 13, 2022
@jeffwidman jeffwidman deleted the make-composer-pin-less-confusing branch September 13, 2022 23:57
@pavera pavera mentioned this pull request Oct 31, 2022
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.

2 participants