-
-
Notifications
You must be signed in to change notification settings - Fork 436
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
Type casting and minor fixes in preparation for declare strict_types=1
#3648
Conversation
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! |
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 |
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've read all the changes and, although some of them don't seem related to the PR to me (adding details on log exception) I think they could be merged, especially because this PR is not enforcing the strict_types, but it's only preparation work.
Agree with @kiatng, to speed things up I've removed from this PR the code that was not related to the strict_types thing. |
app/code/core/Mage/Adminhtml/controllers/Cms/Wysiwyg/ImagesController.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Ng Kiat Siong <kiatsiong.ng@gmail.com>
Co-authored-by: Ng Kiat Siong <kiatsiong.ng@gmail.com>
There's only one point open to fix for this PR. |
declare strict_types=1
`
declare strict_types=1
`declare strict_types=1
since all the comments and subsequent modification, I think this is mergeable |
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