-
Notifications
You must be signed in to change notification settings - Fork 213
Composer backwards compatibility for psr/log #1775
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
base: main
Are you sure you want to change the base?
Composer backwards compatibility for psr/log #1775
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1775 +/- ##
============================================
- Coverage 68.49% 68.28% -0.22%
- Complexity 2970 2979 +9
============================================
Files 448 450 +2
Lines 8714 8734 +20
============================================
- Hits 5969 5964 -5
- Misses 2745 2770 +25
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
6e4d973 to
f66956d
Compare
f66956d to
5b6d269
Compare
Nevay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the other PR:
- running the SDK with partially loaded
composer.phardependencies still seems fragile (for exampleFilesystem::readFile()won't be available when loaded viapost-update-cmdscript) - currently we instrument scripts iff they include
vendor/autoload.php, this feels inconsistent
|
|
||
| if ( | ||
| // Provide a backwards-compatible Psr3 implementation when running under composer. | ||
| class_exists(InstalledVersions::class, false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is InstalledVersions always loaded during composer execution or do we have to drop the autoload: false argument?
| if ( | ||
| // Provide a backwards-compatible Psr3 implementation when running under composer. | ||
| class_exists(InstalledVersions::class, false) | ||
| && InstalledVersions::satisfies(new VersionParser(), 'composer/composer', '~2.0') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must first check whether we are running as composer command, otherwise VersionParser might not be available / composer/composer might not be InstalledVersions::isInstalled().
Additionally this might break setups with composer/composer ^2 and psr/log ^3 installed as project dependencies.
| class_exists(InstalledVersions::class, false) | ||
| && InstalledVersions::satisfies(new VersionParser(), 'composer/composer', '~2.0') | ||
| ) { | ||
| require_once __DIR__ . '/Common/Compatibility/Psr3.php'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this file always be loaded before the SDK is autoloaded?
Should IMO instead live in the SDK autoload script, using just the API package with composer scripts works fine.
Alternative implementation to: #1727