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

removed Mage_Compiler #534

Merged
merged 2 commits into from
Oct 31, 2018
Merged

Conversation

dng-dev
Copy link
Contributor

@dng-dev dng-dev commented Sep 21, 2018

Removed Mage_Compiler #525

@LeeSaferite
Copy link
Contributor

Does it make sense to fix the autoloader at the same time?

It should be returning a false if the class isn't found. That would allow people to easily use composer autoloader in conjunction.

@dng-dev
Copy link
Contributor Author

dng-dev commented Sep 21, 2018 via email

@dng-dev
Copy link
Contributor Author

dng-dev commented Sep 22, 2018

@seansan as mentioned i added the missing return statement for the autoloader.

Copy link
Member

@colinmollenhour colinmollenhour left a comment

Choose a reason for hiding this comment

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

Should Varien_Autoload::registerScope method be kept (doesn't need to do anything) just for BC?

Copy link
Member

@colinmollenhour colinmollenhour left a comment

Choose a reason for hiding this comment

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

Also any reason not to go ahead and replace __autoload with spl_autoload_register since the former is deprecated in PHP 7.2?

@Flyingmana
Copy link
Contributor

the __autoload function can get deleted completely. Since the Varien Autoloader uses the SPL autoloader, this one is never in use.

Flyingmana
Flyingmana previously approved these changes Oct 14, 2018
Copy link
Contributor

@Flyingmana Flyingmana left a comment

Choose a reason for hiding this comment

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

the BC break is I think acceptable.
usage is very low (or hard to find via github) and it fails early, so its fast to notice and easy to fix.
also, when keeping we would maybe also need to keep the other scope related functions...

I suggest to wait in this case, if there is really someone who is using this extensively.

Fixing the autoload function is easy done over a separate PR then afterwards

@colinmollenhour colinmollenhour dismissed stale reviews from Flyingmana and themself via 8507700 October 31, 2018 21:32
@colinmollenhour colinmollenhour merged commit 223d2ee into OpenMage:1.9.3.x Oct 31, 2018
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Dec 6, 2018
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Feb 14, 2019
fplantinet pushed a commit to fplantinet/magento-lts that referenced this pull request Mar 27, 2019
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Apr 1, 2019
rafaelpatro pushed a commit to rafaelpatro/magento-lts that referenced this pull request May 14, 2019
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 22, 2019
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Oct 25, 2019
@sreichel sreichel mentioned this pull request Jun 11, 2020
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants