-
Notifications
You must be signed in to change notification settings - Fork 30
Add prefix to PSR-4 autoloader #108
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
Add prefix to PSR-4 autoloader #108
Conversation
PSR-4 autoloaders with no prefix can hurt performance, especially in large projects with many dependencies. When resolving class names to files, if Composer cannot use a prefix to limit the search paths, it has to search every autoloader with a blank prefix. This change adds a prefix to the autoloader, to avoid this problem. Running `composer diagnose` previously reported this: ~/src/php-sdk % composer diagnose Checking composer.json: WARNING Defining autoload.psr-4 with an empty namespace prefix is a bad idea for performance With this change, `composer diagnose` is happy. I couldn't add tests for this, because it's not a code change. But, I did run the test suite to verify that it everything still passes as expected.
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
Hi @dcloues , thank you for submitting this fix. I'd like to request you to fill out our CLA mentioned here: https://github.com/optimizely/php-sdk/blob/master/CONTRIBUTING.md#contributing-to-the-optimizely-php-sdk Once done, I shall be able to get this in. Thank you again. |
Hi @aliabbasrizvi - thanks so much! Sorry for missing the CLA the first time around - I read the contributing guide and somehow read right past it, even though it's super clear. I just submitted the agreement - thanks again. |
build |
1 similar comment
build |
Thanks @dcloues for sending this in. Changes look good. I am going to merge this in and it will hopefully be available in the next release 🙂. |
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.
LGTM
PSR-4 autoloaders with no prefix can hurt performance, especially in large projects with many dependencies. When resolving class names to files, if Composer cannot use a prefix to limit the search paths, it has to search every autoloader with a blank prefix.
This change adds a prefix to the autoloader, to avoid this problem.
Running
composer diagnose
previously reported this:With this change,
composer diagnose
is happy.I couldn't add tests for this, because it's not a code change. But, I did run the test suite to verify that it everything still passes as expected.