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

Exclude directories when extracting module wordings #26839

Merged
merged 2 commits into from Dec 21, 2021
Merged

Exclude directories when extracting module wordings #26839

merged 2 commits into from Dec 21, 2021

Conversation

sowbiba
Copy link
Contributor

@sowbiba sowbiba commented Dec 2, 2021 •
edited

Questions Answers
Branch? 1.7.8.x
Description? Excludes vendor, lib, tests directories when scanning modules' directories to extract translation catalogue.
Update prestashop/translationtools-bundle to 4.1.0 and php-cs-fixer to 2.19.3
Type? bug fix
Category? BO
BC breaks? yes
Deprecations? no
Fixed ticket? Fixes #24987
How to test? Use a module which has translations in vendor, lib or tests directory and check that these translations doesn't appear on translations interface.
For example, the module https://github.com/sowbiba/translationtesting has dependencies with https://github.com/sowbiba/translationtesting-dependency which has translated strings (https://github.com/sowbiba/translationtesting-dependency/blob/master/views/templates/admin/configure.tpl#L30). Those translated messages may not appear in the BO page
The performances problems described in the issue may not occurred anymore
Possible impacts? -

This change is Reviewable

Breaking Changes

  • LegacyModuleExtractor now requires an extra parameter catalogueExtractExcludedDirectories to define folders to exclude
  • In LegacyModuleExtractor we use specific class for phpExtractor, smartyExtractor and twigExtractor instead of ExtractorInterface, because the excludedDirs feature is in the TraitExtractor and not defined in the Interface

@sowbiba sowbiba requested a review from a team as a code owner December 2, 2021 15:35
@prestonBot prestonBot added 1.7.8.x Branch Bug fix Type: Bug fix BC break Type: Introduces a backwards-incompatible break labels Dec 2, 2021
matks
matks requested changes Dec 3, 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.

PR OK but please add BC break description in PR body

Verify AbstractFileExtractor usage

src/Core/Translation/Storage/Extractor/LegacyModuleExtractor.php Show resolved Hide resolved
matks
matks previously approved these changes Dec 3, 2021
View reviewed changes
@sowbiba sowbiba force-pushed the translations-modules-exclude-directories branch 2 times, most recently from 5bebdea to d984d9c Compare December 6, 2021 13:08
@matks
Copy link
Contributor

matks commented Dec 7, 2021

Re-running CI

matks
matks previously approved these changes Dec 7, 2021
View reviewed changes
atomiix
atomiix previously approved these changes Dec 13, 2021
View reviewed changes
@atomiix atomiix added the Waiting for QA Status: action required, waiting for test feedback label Dec 13, 2021
@Progi1984 Progi1984 added this to the 1.7.8.3 milestone Dec 13, 2021
@Progi1984 Progi1984 linked an issue Dec 13, 2021 that may be closed by this pull request
Translations file scanning should not go to vendor folder #24987
Closed
@khouloudbelguith khouloudbelguith self-assigned this Dec 15, 2021
@khouloudbelguith khouloudbelguith added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Dec 15, 2021
@sowbiba sowbiba removed the Waiting for author Status: action required, waiting for author feedback label Dec 15, 2021
@sowbiba sowbiba force-pushed the translations-modules-exclude-directories branch from d984d9c to f6c8478 Compare December 17, 2021 16:26
@sowbiba sowbiba closed this Dec 20, 2021
@sowbiba sowbiba reopened this Dec 20, 2021
@khouloudbelguith khouloudbelguith added Waiting for QA Status: action required, waiting for test feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Dec 20, 2021
jolelievre
jolelievre approved these changes Dec 20, 2021
View reviewed changes
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

Thanks @sowbiba

@matthieu-rolland matthieu-rolland added the Waiting for QA Status: action required, waiting for test feedback label Dec 20, 2021
@khouloudbelguith
Copy link
Contributor

Hi @sowbiba,

It is ok ✔️
I checked with a test module provided by @sowbiba
https://watch.screencastify.com/v/X6V93hzmvd036tVcsQpK
I checked the test module https://github.com/tswfi/translationtesting
https://watch.screencastify.com/v/l05DiLbuyz8DRioFU5AQ

Thanks!

@khouloudbelguith khouloudbelguith added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Dec 21, 2021
@sowbiba sowbiba merged commit 4f30521 into PrestaShop:1.7.8.x Dec 21, 2021
@sowbiba sowbiba deleted the translations-modules-exclude-directories branch December 21, 2021 16:30
@matks matks added the Needs documentation Needs an update of the developer documentation label Jul 19, 2022
@kpodemski kpodemski removed the Needs documentation Needs an update of the developer documentation label Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthieu-rolland matthieu-rolland matthieu-rolland approved these changes

@jolelievre jolelievre jolelievre approved these changes

@atomiix atomiix atomiix left review comments

@matks matks matks left review comments

@eternoendless eternoendless Awaiting requested review from eternoendless

Assignees

@khouloudbelguith khouloudbelguith

Labels
1.7.8.x Branch BC break Type: Introduces a backwards-incompatible break Bug fix Type: Bug fix QA ✔️ Status: check done, code approved
Projects
None yet
Milestone
1.7.8.3
Development

Successfully merging this pull request may close these issues.

Translations file scanning should not go to vendor folder
10 participants
@sowbiba @matks @khouloudbelguith @eternoendless @matthieu-rolland @atomiix @jolelievre @Progi1984 @kpodemski @prestonBot