search

Ocramius / PackageVersions

:package: Composer addon to efficiently get installed packages' version numbers
MIT License
3.22k stars 67 forks source link

Use vendor/composer/installed.json and fallback to using composer.lock #82

Closed d42ohpaz closed 5 years ago

d42ohpaz commented 5 years ago

Also be more clear in the messages why the exceptions are being thrown so that developers understand what files are being looked in for package versions. In other words, we know to deploy the vendor/composer/installed.json and/or the composer.lock for the project.

Ocramius commented 5 years ago

I'm wondering if relying on installed.json is reliable enough to get rid of the reliance on composer.lock entirely: what is your thought on that?

d42ohpaz commented 5 years ago

I left the composer.lock checks in for backward compatibility in case someone out there is excluding the installed.json. I wanted to add trigger_error() when paths are not found, but didn't want to cause a failure (more like a deprecation-style notice).

Ocramius commented 5 years ago

I'd say this is fine as-is, then. We can focus on getting rid of the exceptions completely in a 2.x release later on, if you are interested.

Ocramius commented 5 years ago

This requires a rebase to see if newer changes clash with it

Ocramius commented 5 years ago

@9ae8sdf76 think this is ready then, or still working on it?

d42ohpaz commented 5 years ago

@Ocramius just finished up addressing your feedback. ready for another review round.

d42ohpaz commented 5 years ago

I’m okay with it if you are.

On Sun, Jun 30, 2019 at 10:19 AM Marco Pivetta notifications@github.com wrote:

@Ocramius commented on this pull request.

In test/PackageVersionsTest/FallbackVersionsTest.php https://github.com/Ocramius/PackageVersions/pull/82#discussion_r298836490 :

@@ -54,9 +54,15 @@ public function testValidVersions() : void

 public function testValidVersionsWithoutComposerLock() : void
 {
  • // This test will always fail if there is nothing in the installed.json
  • // due to this package being installed with composer install --no-dev.
  • if (json_decode(file_get_contents(getcwd() . '/vendor/composer/installed.json', true)) === []) {
  • $this->markTestSkipped('Empty installed.json (possible --no-dev)');

I'd say that the composer.lock is correct here, even if the packages aren't installed.

We can roll with the patch as-is, if you are happy with it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Ocramius/PackageVersions/pull/82?email_source=notifications&email_token=AATJJT2EQ3HYZA4U3ZWQAILP5C6FJA5CNFSM4HEKNAW2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB5BWZOI#discussion_r298836490, or mute the thread https://github.com/notifications/unsubscribe-auth/AATJJT7H6H6RLYHT47YFZE3P5C6FJANCNFSM4HEKNAWQ .

-- "Do not go gentle into that good night. Rage, rage against the dying of the light." — Dylan Thomas