magento-lts
magento-lts copied to clipboard
Prepare declare strict_types=1
Description
This PR allow to add <?php declare(strict_types=1); in all files of OpenMage (@sreichel in #3647). This is only the beginning.
To add it in all files, run find app/ shell/ lib/Mage/ lib/Magento/ lib/Varien/ -name "*.ph*" -type f -exec sed -i '1s/<?php/<?php declare(strict_types=1);/' {} +
Apply this PR and enjoy backend.
To cancel changes: find app/ shell/ lib/Mage/ lib/Magento/ lib/Varien/ -name "*.ph*" -type f -exec sed -i '1s/<?php declare(strict_types=1);/<?php/' {} +
OpenMage 20.2.0, PHP 7.2 / 8.3.
Contribution checklist
- [x] Pull request has a meaningful description of its purpose
- [x] All commits are accompanied by meaningful commit messages
- [x] All automated tests passed successfully (all builds are green)
- [x] Add yourself to contributors list
you need to be very careful with this, there are several situations where it changes the behavior of the code
Yes, I'm not sure to understand all possible impacts, but, for localhost it looks good! And, with them, I found some critical fails of the life!
there are several situations where it changes the behavior of the code
I did not see any changes in code behavoir during my tests. (not finished, but 80%) Either it works, or you see a fatal error. I think its not that hard to add strict types, its just a lot of work and manual testing. (unit tests would be nice here)
Looks good, perhaps extract enhancement for error message in another PR, and add Mage::logException().
Else we can close it.
Agree with @kiatng, to speed things up I've removed from this PR the code that was not related to the strict_types thing.
There's only one point open to fix for this PR.
since all the comments and subsequent modification, I think this is mergeable