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

Reduce amount of strings we need to pull/push with Transifex #5795

Merged
merged 11 commits into from
Jul 6, 2020
Merged

Reduce amount of strings we need to pull/push with Transifex #5795

merged 11 commits into from
Jul 6, 2020

Conversation

bbondy
Copy link
Member

@bbondy bbondy commented Jun 9, 2020 •
edited
Loading

Resolves brave/brave-browser#10186
Sister PR: brave/brave-browser#10560

  • Automatically apply branding transformations for both grd and xtb files.
  • For strings with incognito -> private, people -> profile, etc, we generate an override file for the grds that have those strings.
  • We send up the _override slugs to transifex for translation, I manually applied the old translations to these so we'd have them.
  • We pull down the _override xtb files from Transifex.
  • We combine those override xtb files into the original ones and remove any matching fingerprint.

For the newly generated xtb and grd updates, see this PR: #5998

Submitter Checklist:

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@bbondy bbondy requested a review from mkarolin June 9, 2020 20:39
@bbondy bbondy self-assigned this Jun 9, 2020
@bbondy bbondy force-pushed the l10n-less branch 3 times, most recently from 591eb65 to a48508f Compare June 10, 2020 15:01
@bbondy
Copy link
Member Author

bbondy commented Jun 10, 2020

rebased on latest master

@mkarolin mkarolin requested a review from petemill June 11, 2020 14:40
@bbondy bbondy added the CI/skip Do not run CI builds (except noplatform) label Jun 24, 2020
@bbondy bbondy requested a review from NejcZdovc as a code owner July 3, 2020 15:31
@bbondy bbondy force-pushed the l10n-less branch 2 times, most recently from 60b8d52 to e7aa465 Compare July 3, 2020 15:47
@bbondy bbondy mentioned this pull request Jul 3, 2020
Merged
@bbondy bbondy requested a review from emerick July 3, 2020 18:04
@bbondy
Copy link
Member Author

bbondy commented Jul 3, 2020

This is updated and ready for another review.
The basic idea is that we:

  • Automatically apply branding transformations for both grd and xtb files.
  • For strings with incognito -> private, people -> profile, etc, we generate an override file for the grds that have those strings.
  • We send up the _override slugs to transifex for translation, I manually applied the old translations to these so we'd have them.
  • We pull down the _override xtb files from Transifex.
  • We combine those override xtb files into the original ones and remove any matching fingerprint.

The actual XTB, GRD, and GRDP files are in another PR that is against the l10n-less branch.

emerick
emerick approved these changes Jul 3, 2020
View reviewed changes
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM, just had one question about a line that looked redundant/incorrect.

script/lib/grd_string_replacements.py
Comment on lines +38 to +39
(r'Bookmarks Bar\n', r'Bookmarks\n'),
(r'Bookmarks bar\n', r'Bookmarks\n'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these two lines in conflict with each other? Or maybe I'm not understanding how this works...

Copy link
Member Author

Choose a reason for hiding this comment

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

just case sensitive replacement. Bar vs bar. Could be done with a regex probably too

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, missed that!

@bbondy bbondy mentioned this pull request Jul 3, 2020
Merged
29 tasks
@bbondy bbondy removed the CI/skip Do not run CI builds (except noplatform) label Jul 3, 2020
bbondy added 7 commits July 3, 2020 15:26
@bbondy
Pull from Local xtbs when possible
ab1a185 
...instead of using Transifex

These string slugs are no longer pushed nor pulled from Transifex:
brave_strings
components_brave_strings
components_strings
generated_resources
android_chrome_strings
@bbondy
Generate override files for strings we override
4e0553d
@bbondy
Preserve file elements which point to xtb in override grds
5779f6c 
This is so that it's more compatible with existing functions in
transifex.py
@bbondy
Send up override files to Transifex
8ba80c0
@bbondy
If there are any modified parts or messages include strings
0a6f323
@bbondy
No more need to check for missing Chromium strings
b3a42e1 
This commit also includes code to update old FPs before the branding replacement to new FPs
@bbondy
Recombine override files into xtb files in py
31b1416
@bbondy bbondy added this to the 1.12.x - Nightly milestone Jul 3, 2020
mkarolin
mkarolin approved these changes Jul 6, 2020
View reviewed changes
Copy link
Collaborator

@mkarolin mkarolin left a comment

Choose a reason for hiding this comment

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

++

bbondy added 2 commits July 6, 2020 12:00
@bbondy
Add generated override grd and grdp files
4a6ceb8 
... and removals for C84 minor bump grd differences
@bbondy
Update XTB files
862435f
@bbondy bbondy merged commit 574a6b9 into master Jul 6, 2020
@bbondy bbondy deleted the l10n-less branch July 6, 2020 16:25
@mkarolin mkarolin mentioned this pull request Jul 6, 2020
Merged
32 tasks
@mkarolin mkarolin mentioned this pull request Jul 7, 2020
Merged
32 tasks
mkarolin pushed a commit that referenced this pull request Jul 7, 2020
@bbondy @mkarolin
Merge pull request #5795 from brave/l10n-less
06e41cd 
Reduce amount of strings we need to pull/push with Transifex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emerick emerick emerick approved these changes

@mkarolin mkarolin mkarolin approved these changes

@petemill petemill Awaiting requested review from petemill

@NejcZdovc NejcZdovc Awaiting requested review from NejcZdovc

Assignees

@bbondy bbondy

Labels
None yet
Projects
None yet
Milestone
1.12.x - Release #1
Development

Successfully merging this pull request may close these issues.

Reduce amount of strings we need to pull/push with Transifex
3 participants
@bbondy @emerick @mkarolin