Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add ability to exclude directories when extracting #87

Merged
merged 7 commits into from Nov 19, 2021
Merged

Add ability to exclude directories when extracting #87

merged 7 commits into from Nov 19, 2021
+448 −148

Conversation

sowbiba
Copy link
Contributor

@sowbiba sowbiba commented Jun 16, 2021 •
edited

Questions Answers
Description? Allow to set directories that will be excluded when scanning folders for catalogue extraction.
Type? improvement
BC breaks? yes
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#24987
How to test? See PrestaShop/PrestaShop#24987
Possible impacts? -

@sowbiba sowbiba force-pushed the exclude-directories branch 2 times, most recently from 3235b33 to 1e97b68 Compare June 16, 2021 18:49
matks
matks reviewed Jun 18, 2021
View reviewed changes
Copy link
Contributor

@matks matks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code seems OK ✅

1 nice-to-have and 1 maybe typo

Also it could be interesting to also run tests with translation wordings that have parameters, such as

  • The file is too large. The maximum size allowed is [1] MB. The file you are trying to upload is [2] MB.
  • Are you sure you want to delete the selected image?|Are you sure you want to delete the %filesNb% selected images?

Translation/Extractor/AbstractFileExtractor.php Outdated Show resolved Hide resolved
Tests/resources/directory/root_template.tpl Outdated Show resolved Hide resolved
@matks
Copy link
Contributor

matks commented Jul 22, 2021

🆙 @sowbiba 😉

@sowbiba sowbiba force-pushed the exclude-directories branch 2 times, most recently from 59529b8 to bb07866 Compare July 26, 2021 20:12
atomiix
atomiix previously requested changes Jul 28, 2021
View reviewed changes
Twig/BaseLexer.php Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@sowbiba sowbiba force-pushed the exclude-directories branch 2 times, most recently from f4d6e7c to 374a4a3 Compare August 24, 2021 12:28
@sowbiba sowbiba mentioned this pull request Oct 6, 2021
Closed
@sowbiba sowbiba requested a review from atomiix November 9, 2021 14:44
eternoendless
eternoendless reviewed Nov 16, 2021
View reviewed changes
Copy link
Member

@eternoendless eternoendless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final comments, after that it's good to merge!

@eternoendless
Copy link
Member

Thank you @sowbiba

@eternoendless eternoendless merged commit b4ebf59 into PrestaShop:master Nov 19, 2021
@matks
Copy link
Contributor

matks commented Nov 22, 2021

Great job @sowbiba 👍

@sowbiba sowbiba deleted the exclude-directories branch November 29, 2021 10:36
@sowbiba sowbiba mentioned this pull request Nov 30, 2021
Closed
@matks matks added this to the 5.0.1 milestone Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eternoendless eternoendless eternoendless approved these changes

@atomiix atomiix Awaiting requested review from atomiix

@PierreRambaud PierreRambaud Awaiting requested review from PierreRambaud

@matks matks Awaiting requested review from matks

@Progi1984 Progi1984 Awaiting requested review from Progi1984

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
5.0.1
Development

Successfully merging this pull request may close these issues.

Translations file scanning should not go to vendor folder
6 participants
@sowbiba @matks @eternoendless @PierreRambaud @Progi1984 @atomiix