-
Notifications
You must be signed in to change notification settings - Fork 687
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
build: eliminate i18n_tool.py
#6954
Conversation
4241fd7
to
99d4eca
Compare
@nathandyer, I'd love your thoughts on this "test" plan, which is really a mock (compressed) QA/localization/release/deployment cycle. Feel free to leave comments here, and we can discuss this (and next week's timeline) in Monday's stand-up too. |
@cfm I think this "test" plan makes a lot of sense, and is well thought out! I'm looking forward to pairing next week to work through this mock workflow together! |
@nathandyer, now that we've postponed the testing session to Thursday, would you be able to run through the async portion of the test plan beforehand? (The very beginning of the "Test Plan" section, before the "Peform a Mock Release..." subsection.) It would be nice to go into the end-to-end operational testing having already done a preliminary review of the self-contained functionality. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(very drive-by review)
This list was *almost* in alphabetical order, so I've reordered it. If there's a logical order in which we want these targets to run, we should document that.
As @legoktm notes[1] on a previous version of this commit, it's a single line whether or not we wrap it in a Makefile target. [1]: #6954 (comment)
39c2659
to
37a79f9
Compare
As @legoktm notes[1] on a previous version of this commit, it's a single line whether or not we wrap it in a Makefile target. [1]: #6954 (comment)
@nathandyer, all the |
…-sort-output" I'm not sure what about the securedrop/desktop sources would otherwise introduce sorting drift between my local environment and CI, but there's no downside to applying this sorting wherever it turns out to be necessary.
6afba25
to
1735fa2
Compare
Okay, we're out of the woods with 1735fa2. @nathandyer, this was truly a lint-level fix, the gettext equivalent of re-running |
Thank you @cfm! I took this for a spin this morning, but it doesn't seem to be working as we might have hoped. The process I followed was:
Result: |
@nathandyer and I discussed #6954 (comment) out of band and concluded that it's mostly passing:
I'll confirm this in a moment on my end. I'll also rebase my own local copy of |
Confirmed here:
|
Confirmed that freedomofpress/securedrop-i18n-sandbox@9b09f371a98522f5120b60996f63c837460fa264 + #6975 includes https://github.com/weblate-sandbox-fpf/securedrop-i18n-sandbox/blob/73b5aea826e6e6ec7a8f4ebe0f4c9994b183cce0/securedrop/translations/es_ES/LC_MESSAGES/messages.po#L866-L867. root@sd-staging:~/securedrop-i18n-sandbox# git show 55a37b831206bfbfdffe3b57babec678166f1cfb
commit 55a37b831206bfbfdffe3b57babec678166f1cfb (HEAD -> i18n)
Merge: 9b09f371a d431061c7
Author: root <root@(none)>
Date: Tue Oct 10 23:13:34 2023 +0000
Merge remote-tracking branch 'main/develop' into i18n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything is looking great to me, and I'm satisfied that our test plan (as unusual as it was, given that the author of the PR was directly involved in the testing itself) is sufficient and covers all the necessary bases. From my perspective, we're OK to go ahead and merge, then move this forward.
Thanks, @nathandyer! I'm about to merge and begin the fiddly bits between here and Weblate. |
---for the greatest coverage as soon as possible.[1] [1]: #6954 (comment)
Since #6954 and freedomofpress/securedrop-dev-docs#86, we no longer derive translator credits from Git history at all, so this file is unused as well as out of date.
Status
Ready for review
Description of Changes
Closes #6917 by removing
i18n_tool.py
in favor of:make extract-strings
, which wrapspybabel extract
;make check-{desktop-files,strings}
, which enforcesmake extract-strings
in CI;pybabel compile
;make supported-locales
, which consumesi18n.json
and can be further processed viajq
;l10n.txt
include with link toi18n.rst
securedrop-docs#503; andTesting
Because this pull request offloads much of our existing localization workflow onto Weblate, very little self-contained, local testing is possible or worthwhile. Namely:
develop
and here, with a fresh build image each time:(docker|podman) image prune -a --force && make build-debs
ondevelop
and save them (e.g.,cp -Rp build build@develop
).(docker|podman) image prune -a --force && make build-debs
on this branch.diffoscope build@develop/focal/securedrop-app-code*.deb build/focal/securedrop-app-code*.deb
shows only:.mo
filesInstead...
Perform a mock release and deployment via
securedrop-i18n-sandbox
A mock Localization Manager (@cfm) and Release Manager (@nathandyer) should pair their way through (approximately) the following:
weblate-sandbox.securedrop.org
uses...freedomofpress/securedrop-i18n-sandbox
.weblate-sandbox.securedrop.org
, configuresecuredrop/securedrop
andsecuredrop-desktop
(fromfreedomofpress/securedrop-i18n-sandbox
) likesecuredrop/securedrop-client
(fromfreedomofpress/securedrop-client-i18n-sandbox
). Add-ons:desktop
only: Generate MO filessecuredrop-i18n-sandbox@i18n
(as though it were already merged intosecuredrop@develop
).securedrop-i18n-sandbox
, make, commit, and push a change to a source string...securedrop/
install_files/ansible-base/roles/tails-config/templates/locale/
weblate-sandbox.securedrop.org
, make, commit, and push a change to a translation...securedrop/securedrop
securedrop/desktop
securedrop-i18n-sandbox
, review and merge the pull request.make dev
insecuredrop-i18n-sandbox@i18n
contains the current translations fromsecuredrop/securedrop
.securedrop-i18n-sandbox
contains the current translations fromsecuredrop/securedrop
.securedrop-i18n-sandbox
contains the current translations fromsecuredrop/desktop
.Along the way, append
fixup
commits both here and in freedomofpress/securedrop-dev-docs#86 as needed for both a working implementation and correct documentation, respectively, of this workflow.fixup
s (only) before the final approval and merge.Deployment
Before merging:
weblate.securedrop.org
, lock thesecuredrop/securedrop
andsecuredrop/desktop
components.weblate.securedrop.org
uses...freedomofpress/securedrop
.After merging:
weblate.securedrop.org
, configuresecuredrop/securedrop
andsecuredrop-desktop
(fromfreedomofpress/securedrop
) likesecuredrop/securedrop-client
(fromfreedomofpress/securedrop-client
). Add-ons: as above.weblate.securedrop.org
, unlock thesecuredrop/securedrop
andsecuredrop/desktop
components.securedrop
.Checklist
If you made changes to the server application code:
make lint
) and tests (make test
) pass in the development containerIf you made non-trivial code changes:
Choose one of the following: