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

Improve i18n_tool.py list-translators, update-from-weblate #5571

Merged
merged 1 commit into from
Nov 19, 2020

Conversation

rmol
Copy link
Contributor

@rmol rmol commented Oct 9, 2020

Status

Ready for review

Description of Changes

Fixes #5570.

Allow the specification of a commit whose timestamp will be used as the starting point for gathering translator credits, instead of
starting from the last commit that says "l10n: sync". That breaks badly if there are any source string changes during the release cycle that require another sync, so instead, just examine all the commits since the time of the specified revision, defaulting to the last release tag.

Also, instead of always gathering translator contributions up to the tip of i18n/i18n for list-translators and update-from-weblate, allow the specification of a commit. This is intended to allow verification of these functions' results; release management tasks should use the default target of the i18n/i18n branch tip.

Testing

Verify that i18n_tool.py update-from-weblate produces the same translations it did for 1.6.0

  • Create a test-i18n-merge branch starting at develop as of the creation of the i18n-merge branch for 1.6.0:

    git checkout -b test-i18n-merge 8b50a9e3c

  • Check out i18n_tool.py from the improve-list-translators branch:

    git checkout improve-list-translators -- securedrop/i18n_tool.py && git commit -m 'get new i18n_tool.py'

  • Activate your virtualenv and update the translations from the i18n/i18n branch at the same commit used for 1.6.0:

    python3 securedrop/i18n_tool.py update-from-weblate --target d34ba2a576bf7ef3faf25fb3e903a492ddff0bac

  • Verify that the translations match 1.6.0:

    git diff --stat origin/release/1.6.0 -- securedrop/translations

    No differences should be reported.

Verify the output of i18n_tool.py list-translators

  • Run python3 securedrop/i18n_tool.py list-translators --target d34ba2a576bf7ef3faf25fb3e903a492ddff0bac --since 1.5.0 and compare the output to the contributors listed in the commit messages from running git log on the test-i18n-merge branch back to the commit cherry-picking i18n_tool.py.

  • Run python3 securedrop/i18n_tool.py list-translators. The translators credited should match the authors of commits since the i18n commit that went into 1.6.0, which you can obtain with git log d34ba2a576bf7ef3faf25fb3e903a492ddff0bac..i18n/i18n.

Deployment

A bug here could result in translations not making it to production, so the localization managers for 1.7.0 should give the results of update-from-weblate and list-translators extra scrutiny.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

If you made non-trivial code changes:

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

@rmol rmol force-pushed the improve-list-translators branch from 6a1b548 to 13ce2de Compare October 9, 2020 18:19
@rmol rmol force-pushed the improve-list-translators branch 2 times, most recently from dd48337 to cc06bf7 Compare October 16, 2020 14:09
@rmol rmol force-pushed the improve-list-translators branch from cc06bf7 to 5fd0c69 Compare October 26, 2020 21:13
@rmol rmol force-pushed the improve-list-translators branch 4 times, most recently from 8548a74 to eda32da Compare November 9, 2020 15:56
@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2020

This pull request introduces 1 alert when merging eda32da into ce22827 - view on LGTM.com

new alerts:

  • 1 for Unused import

@rmol rmol force-pushed the improve-list-translators branch 2 times, most recently from 73a5a22 to 3c85c6f Compare November 9, 2020 18:15
Allow the specification of a commit whose timestamp will be used as
the starting point for gathering translator credits, instead of
starting from the last commit that says "l10n: sync". That breaks
badly if there are any source string changes during the release cycle
that require another sync, so instead, just examine all the commits
since the time of the specified revision, defaulting to the last
release tag.

Also, instead of always gathering translator contributions up to the
tip of i18n/i18n for list-translators and update-from-weblate, allow
the specification of a commit. This is intended to allow verification
of these functions' results; release management tasks should use the
default target of the i18n/i18n branch tip.
@rmol rmol force-pushed the improve-list-translators branch from 3c85c6f to 79640ee Compare November 9, 2020 18:44
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.

The code change looks good. but, I had some trouble in following the test steps:

❯ git checkout -b test-i18n-merge 8b50a9e3c                        
Switched to a new branch 'test-i18n-merge' 
❯ git checkout improve-list-translators -- securedrop/i18n_tool.py && git commit -m 'get new i18n_tool.py'
fatal: invalid reference: improve-list-translators 

@rmol Should I manually copy paste and test? (I guess yes)

@rmol
Copy link
Contributor Author

rmol commented Nov 18, 2020

@kushaldas Thanks for looking, and sorry about that command -- if you don't already have the branch checked out locally, the command should be: git checkout origin/improve-list-translators -- securedrop/i18n_tool.py && git commit -m 'get new i18n_tool.py'.

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.

The code change looks good. Tested with the following instructions:

Testing

Verify that i18n_tool.py update-from-weblate produces the same translations it did for 1.6.0

  • Create a test-i18n-merge branch starting at develop as of the creation of the i18n-merge branch for 1.6.0:

    git checkout -b test-i18n-merge 8b50a9e3c

  • Check out i18n_tool.py from the improve-list-translators branch:

    git checkout improve-list-translators -- securedrop/i18n_tool.py && git commit -m 'get new i18n_tool.py'

  • Activate your virtualenv and update the translations from the i18n/i18n branch at the same commit used for 1.6.0:

    python3 securedrop/i18n_tool.py update-from-weblate --target d34ba2a576bf7ef3faf25fb3e903a492ddff0bac

  • Verify that the translations match 1.6.0:

    git diff --stat origin/release/1.6.0 -- securedrop/translations

    No differences should be reported.

Verify the output of i18n_tool.py list-translators

  • Run python3 securedrop/i18n_tool.py list-translators --target d34ba2a576bf7ef3faf25fb3e903a492ddff0bac --since 1.5.0 and compare the output to the contributors listed in the commit messages from running git log on the test-i18n-merge branch back to the commit cherry-picking i18n_tool.py.

  • Run python3 securedrop/i18n_tool.py list-translators. The translators credited should match the authors of commits since the i18n commit that went into 1.6.0, which you can obtain with git log d34ba2a576bf7ef3faf25fb3e903a492ddff0bac..i18n/i18n.

@kushaldas kushaldas merged commit 315b470 into develop Nov 19, 2020
@kushaldas kushaldas deleted the improve-list-translators branch November 19, 2020 09:45
@eloquence eloquence added this to the 1.7.0 milestone Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

i18n_tool.py list-translators is broken by multiple syncs in a release
3 participants