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 unused ProductController::assignAttributesCombinations() method and call #36056
base: develop
Are you sure you want to change the base?
Removed unused ProductController::assignAttributesCombinations() method and call #36056
Conversation
Hi, thanks for this contribution! I found some issues with the Pull Request description:
Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much! (Note: this is an automated message, but answering it will reach a real human) |
This could be used by some templates to list all possible variants. 🤔 |
I can (hardly) imagine themes created for 1.6 and then upgraded all the way to 8.1.x. |
Hello @the-ge While I could agree that this method has been rarely used, we cannot just remove it, we need to deprecate it first, and then remove it in the next major release. |
Well, the method is used by the core itself on every product page creation. What's not used is its work result. Deprecating this method will only fill the log with deprecation messages telling the core developers to stop using it. Am I missing something? |
@the-ge that's why I like your fix from separate PR better, it fixes the problem while keeping everything working |
Hmm, the way I'm seeing them now, these are the choices:
Well, I do not really know why a theme with v1.6 legacy code should be protected. From my point of view, the PrestaShop theme market state is not that good - saturated with builder-based monsters creating multi-MB JavaScript and, worse, multi-MB CSS pages with abysmal performance. Choosing legacy instead of performance only adds to this state. |
ProductController::assignAttributesCombinations()
method assigns an array of attribute combinations (with size being the count of attributes x count of product combinations, i.e. 4 x 5000 in my case) as a Smarty template variable. As the count of attributes and product combinations increase, this consumes more and more resources. Recent improvements directed at reducing resources consumed are #34453 and #36039. The thing is, the template variable this method assigns seems to have been last used in PrestaShop v1.6, so the method and the call to it should be removed altogether.