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

Reduce localization manager review noise #5791

Merged
merged 2 commits into from
Feb 18, 2021
Merged

Reduce localization manager review noise #5791

merged 2 commits into from
Feb 18, 2021

Conversation

rmol
Copy link
Contributor

@rmol rmol commented Feb 15, 2021

Status

Ready for review

Description of Changes

Adjust i18n_tool.py to reduce translation review noise:
- Stop including the locations of source strings in the .po files.
- Stop wrapping source strings in both .pot and .po files.
- Sort the translated names in the admin workstation desktop icon files.

I asked the translators whether they found source code locations useful. Some have tried to understand the context of a source string by looking at the code, but they didn't get the locations from Weblate. They would all prefer context be provided via screenshots or other means: rummaging through our source code is a last resort.

Fixes #5769.
Fixes #5426

Testing

It would be best to compare the result of these changes if #5790 were merged to develop first. Once the messages.pot file on develop has been brought up to date, you can follow the test plan for #5790, but checking out this branch (reduce-lm-review) instead of update-translation-messages. You should only ever see the same 13 untranslated alt text strings in Hindi: the output on this branch (which has dropped source code locations and line wrapping) should match the output on develop.

Once the web messages have been checked, verify that the desktop icon templates contain the same names, just sorted. Take a look to confirm the sorting, then run this to confirm that the same translated names are present on both develop and this branch:

  • git checkout reduce-lm-review
  • for i in source journalist; do diff -u <(sort install_files/ansible-base/roles/tails-config/templates/desktop-${i}-icon.j2) <(git show develop:install_files/ansible-base/roles/tails-config/templates/desktop-${i}-icon.j2|sort) && echo "names are the same in the ${i} template"; done

You should see:

names are the same in the source template
names are the same in the journalist template

Deployment

The actual translated strings should not change at all. This just makes visual review of the .po(t) files easier.

Checklist

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

Choose one of the following:

  • I have opened a PR in the docs repo for these changes, or will do so later
  • I would appreciate help with the documentation
  • These changes do not require documentation

@rmol rmol added i18n Anything related to translation or internationalization of SecureDrop goals: speed up release process labels Feb 15, 2021
@rmol rmol added this to the 1.8.0 milestone Feb 15, 2021
@kushaldas kushaldas self-assigned this Feb 16, 2021
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

Looks good, I will merge after a chat with @rmol (that is after the rebase).

kushaldas
kushaldas previously approved these changes Feb 16, 2021
Stop including the locations of source strings in the .po files. Stop
wrapping source strings in both .pot and .po files. Sort the
translated names in the admin workstation desktop icon files.
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

make translate on d13377e yields no changes other than the desktop files, when diffed with cceb323 , and visual review of the i18n tool looks good to me, thanks @rmol

@emkll emkll merged commit b56c6b0 into develop Feb 18, 2021
@emkll emkll deleted the reduce-lm-review branch February 18, 2021 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goals: speed up release process i18n Anything related to translation or internationalization of SecureDrop
Projects
None yet
3 participants