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

Add filter on the modules folder to avoid caching all files by Twig #12208

Merged
merged 2 commits into from Jan 31, 2019

Conversation

Projects
None yet
7 participants
@Quetzacoalt91
Copy link
Member

Quetzacoalt91 commented Jan 18, 2019

Questions Answers
Branch? develop
Description? Starting with PrestaShop 1.7.5.0, cache generation is horribly long and create a huge load of files. This is caused by the new path added to the twig configuration, which makes him parse all the modules/ folder and cache every file it can find (even css, js, md files). This PR extends the twigBundle classes to filter by extension the modules folder.
Type? improvement
Category? CO
BC breaks? Nope
Deprecations? Nope
Fixed ticket? /
How to test? Modules with Twig templates still work, with the same namespace.

This change is Reviewable

@Quetzacoalt91 Quetzacoalt91 force-pushed the Quetzacoalt91:avoig-twig-caching-all-files-non-bc branch from 6138c63 to 4ee725f Jan 18, 2019

use Twig\Environment;
use Twig\Loader\ArrayLoader;
// REVIEW ME: Is this class well located?

This comment has been minimized.

@Quetzacoalt91

This comment has been minimized.

@matks

matks Jan 22, 2019

Contributor

Sorry for the late answer 😞

Usually we mirror the class location / namespace for its unit test, so it would locate this test in namespace PrestaShopBundle/Cache/ rather. Do you wish to drop the PrestaShopBundle for Core ?

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Jan 22, 2019

Author Member

Okay, let's strictly match the namespaces.

@Quetzacoalt91 Quetzacoalt91 force-pushed the Quetzacoalt91:avoig-twig-caching-all-files-non-bc branch from 4ee725f to 88428fd Jan 19, 2019

@mickaelandrieu

This comment has been minimized.

Copy link
Contributor

mickaelandrieu commented Jan 22, 2019

/!\ I think we should merge this one in 1.7.5.1 too, WDYT @eternoendless?

use PHPUnit\Framework\TestCase;
use PrestaShopBundle\Twig\Locator\ModuleTemplateIterator;
// REVIEW ME: Is this class well located?

This comment has been minimized.

@matks

matks Jan 22, 2019

Contributor

Same here, I'd expect this test to be in unit test folder PrestaShopBundle/Twig/Locator/ (that would be tests/Unit/PrestaShopBundle/Twig/Locator/ folder) ?

@PierreRambaud
Copy link
Contributor

PierreRambaud left a comment

See comments from the team

@PierreRambaud PierreRambaud added this to the 1.7.6.0 milestone Jan 24, 2019

@Quetzacoalt91

This comment has been minimized.

Copy link
Member Author

Quetzacoalt91 commented Jan 24, 2019

@PierreRambaud, it seems you missed the label Outdated, the changes were already done.

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Jan 24, 2019

I don't add the Waiting for Qa label since we need to know if we add this in 1.7.5.x :)

@kpodemski

This comment has been minimized.

Copy link
Contributor

kpodemski commented Jan 29, 2019

Well done, looking forward to have it in 1.7.5.1 :)

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Jan 29, 2019

Thinking it will not be added in the 1.7.5.1. Adding the Waiting for QA label.

@mbadrani mbadrani self-assigned this Jan 29, 2019

@mbadrani

This comment has been minimized.

Copy link
Contributor

mbadrani commented Jan 29, 2019

@Quetzacoalt91 do you have an example of module using twig template except ps_linklist ?
it's ok for the namespace by the way

@Quetzacoalt91

This comment has been minimized.

Copy link
Member Author

Quetzacoalt91 commented Jan 29, 2019

Yep, @mickaelandrieu has a module which uses the twig templates and compatible from PrestaShop 1.7.4.

@mbadrani

This comment has been minimized.

Copy link
Contributor

mbadrani commented Jan 30, 2019

@mickaelandrieu waiting for your module

@Quetzacoalt91

This comment has been minimized.

Copy link
Member Author

Quetzacoalt91 commented Jan 30, 2019

@mbadrani mbadrani added QA ✔️ and removed waiting for QA labels Jan 30, 2019

@Quetzacoalt91 Quetzacoalt91 merged commit cff5ce6 into PrestaShop:develop Jan 31, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Quetzacoalt91 Quetzacoalt91 deleted the Quetzacoalt91:avoig-twig-caching-all-files-non-bc branch Jan 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment