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.

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

Exclude non-native files when extracting localization strings #3493

Merged
merged 1 commit into from
May 13, 2021

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented May 12, 2021

Fixes the case of extra strings of #3466.

Description

I added the following paths to the excluded argument of the make:pot commands, this way we prevent adding extra strings from non-native files.

New excluded paths:

  • build/*
  • build-module/*
  • build-style/*
  • *.js
  • *.php

In theory, this change shouldn't be required because the --include argument should only include the specified paths, however, this is not working as expected.

Besides, just by excluding *.js should only extract strings from *.native.js files, unfortunately, it's not and it's including by default the build* folders 🤷‍♂️ .

To test

Extra strings are not added

  1. Run command npm run genstrings
  2. Observe that the localization strings files are not including a high number of strings

Only extract strings from native files

  1. Run command npm run makepot:ios -- --debug
  2. Observe the output log and verify that only .native.js files are being parsed

Note: Some files are giving parse failures, this is caused by the use of the optional chaining operator that is not supported by the script that extracts the strings.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@fluiddot fluiddot added [Type] Bug Something isn't working release-process labels May 12, 2021
@fluiddot fluiddot added this to the 1.53.0 (17.4) milestone May 12, 2021
@fluiddot fluiddot self-assigned this May 12, 2021
@fluiddot fluiddot marked this pull request as ready for review May 13, 2021 08:26
@fluiddot fluiddot requested review from dcalhoun and mchowning May 13, 2021 08:29
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

LGTM. I verified that in this branch npm run genstrings generated significantly fewer translations string changes and npm run makepot:ios -- --debug only included *.native.js files in comparison to the develop branch.

@fluiddot fluiddot merged commit 0be9bd8 into develop May 13, 2021
@fluiddot fluiddot deleted the fix/prevent-extra-localization-strings branch May 13, 2021 13:58
@AmandaRiu AmandaRiu mentioned this pull request May 14, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-process [Type] Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants