-
-
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
set phpstan to bleedingEdge config #3011
Conversation
Nice, I noticed an improvement from 1:35 to only 40s when using the new configuration! |
Fixed some phpstan issues, but some tricky ones popped up. |
app/code/core/Mage/Adminhtml/Block/Catalog/Category/Abstract.php
Outdated
Show resolved
Hide resolved
app/code/core/Mage/Adminhtml/Block/Catalog/Category/Abstract.php
Outdated
Show resolved
Hide resolved
fixed conflict with baseline by using 1.9.4.x baseline |
guys, this is so incredibly much faster that I suggest we regenerate the baseline based on the code that's now in the PR and merge it. what do you think? |
a21f045
to
3cb3c63
Compare
Resolved some issues that were newly added to baseline since they were easy to fix. |
app/code/core/Mage/Adminhtml/Block/Catalog/Category/Abstract.php
Outdated
Show resolved
Hide resolved
app/code/core/Mage/Adminhtml/Block/Catalog/Category/Abstract.php
Outdated
Show resolved
Hide resolved
app/code/core/Mage/Adminhtml/Block/Catalog/Category/Abstract.php
Outdated
Show resolved
Hide resolved
app/code/core/Mage/Adminhtml/Block/Catalog/Category/Abstract.php
Outdated
Show resolved
Hide resolved
@elidrissidev I lost your last comment, I guess that change was an older one (since this PR went thru many revisions), I've removed it, let's see if it works |
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.
Looks good
merged and v20ed |
btw when you clear the description of the final squashed commit, be sure to keep the any special lines in it such as |
sorry, sometimes there's so much stuff in there that doesn't seem useful at all :-( |
No problem :) |
there is a performance improvement for phpstan possible when switching to the currend bleedingEdge config.
https://phpstan.org/blog/phpstan-1-6-0-with-conditional-return-types#lower-memory-consumption
Heard about it while researching for a different project ( phpstan/phpstan#8813 )
Questions or comments
There are some incompatibilities mentioned, and while I checked I also saw that we get some errors.
Not sure yet about what needs to change, but for the future we will need to fix it anyway anywhen
Contribution checklist (*)