search

classyllama / ClassyLlama_AvaTax

This extension has been deprecated in favor of https://github.com/avadev/Avalara-AvaTax-for-Magento2
Open Software License 3.0
23 stars 15 forks source link

Fixed BP constant issue in registration.php #102

Closed makao closed 6 years ago

makao commented 6 years ago

This PR fixes issue when running CLI tools like PHPUnit, PHPMD or PHPCS. Composer is autoloading registration.php, but app/autoload.php isn't loaded there so BP constant is not available.

To reproduce try to run

php bin/magento dev:tests:run unit

It will throw an error

PHP Notice:  Use of undefined constant BP - assumed 'BP' in .../vendor/classyllama/module-avatax/registration.php on line 12
erikhansen commented 6 years ago

@makao I can't merge your PR as it stands, as it will cause an issue. Take a look at this commit and you'll see that your PR actually is an exact revert of the changes you made: https://github.com/makao/ClassyLlama_AvaTax/commit/30403feadb57f9ff13fffa1e2076d4eed07343f1#diff-f9a20ab0f1b66092b9c7efa82de1f220

I actually ran into a similar issue in my fork of this repo. In this fork, I was working on getting automated tests running with Travis CI. This is how I dealt with this issue: https://github.com/erikhansen/ClassyLlama_AvaTax/blob/feature/travis-ci-setup/registration.php

If you update this PR include the contents of the registration.php file, we can merge this PR.

makao commented 6 years ago

I am not sure what do you mean by it will cause an issue. I've tried to compile DI and everything worked. However I've updated with check if BP is defined.

erikhansen commented 6 years ago

@rsisco I'm quite certain this PR will work without causing any issues, but can you test locally and include these changes in your next release?

molsm commented 6 years ago

Hello @makao and @erikhansen

Any updates on this? We are getting this issue too. Would be super cool to get this fix as fast as possible.

Best regards,

makao commented 6 years ago

Still waiting, I've fixed it locally for development.

rsisco commented 6 years ago

@makao - This has been included in the latest release - 1.3.1. Thanks!