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

Update translations #3013

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Update translations #3013

wants to merge 1 commit into from

Conversation

lbschenkel
Copy link
Collaborator

@lbschenkel lbschenkel commented Jul 10, 2024

  • update exiv2.pot ('update-pot' build target)
  • sync translations with Crowdin
  • sorted files to make them more diffable (helps with future PRs)

@lbschenkel
Copy link
Collaborator Author

I updated the .pot file first (there were strings in the code not present in the .pot file), I will upload it to Crowdin and I will rebuild the .po files.

@kmilos
Copy link
Collaborator

kmilos commented Jul 10, 2024

Thanks, we can do it this way as well, will close my PR.

Please update LINGUAS and CMakeLists.txt for new languages as well (it, ka, and pt_BR).

P.S. One gripe w/ Crowdin - it seems to nuke all the comments and historical information in the po files...

@kmilos kmilos added the L10n Translation languages label Jul 10, 2024
@lbschenkel
Copy link
Collaborator Author

lbschenkel commented Jul 10, 2024

I've updated all files now.

Note that the current workflow (or rather: the lack of it) can lead to accidental loss of translations. The reason is that Crowdin is supposed to be the source of truth for them, so if there's any PR coming where translators are submitting new translations (changes to .po files) then those need at the very least to be uploaded to Crowdin.

I did some sanity checks by looking at the commit logs and cross-checking with Crowdin, so I think that's not happening in this particular case. But with any manual process, especially for the unfamiliar, that's bound to happen.

Ideally this is what should happen:

  • any commit PR touching translatable strings should also regenerate the .pot file and include that with the commit PR (or at the very least trigger a new commit/PR that does that)
  • changes to .pot files should trigger an upload of that file to Crowdin (so those strings become visible to translators)
  • periodically we trigger a sync with Crowdin (automated, just the triggering is manual) which will result in a PR with translation updates — triggering could also be automated, for example after any merges
  • we should not accept direct PRs with .po changes, or at the very least those will need to be paired with an upload of those changes to Crowdin so those translations don't get lost in the next sync.

I can help setting that up, however I need the buy-in from the maintainers.

@lbschenkel
Copy link
Collaborator Author

lbschenkel commented Jul 10, 2024

P.S. One gripe w/ Crowdin - it seems to nuke all the comments and historical information in the po files...

Yes, that's not ideal. In a previous job I was the internationalization expert, so I didn't use the PO files as-is but I built tooling that merged those with the original PO files and kept them sorted as well, so it's more diffable.

Unfortunately I don't think I can dedicate enough time for this project to replicate that tooling here.

@kmilos
Copy link
Collaborator

kmilos commented Jul 10, 2024

P.S. One gripe w/ Crowdin - it seems to nuke all the comments and historical information in the po files...

Yes, that's not ideal.

To the point that makes me hesitate merging this on main, as it would nuke credits to past contributors, let alone some useful comments...

@lbschenkel
Copy link
Collaborator Author

lbschenkel commented Jul 10, 2024

Please update LINGUAS and CMakeLists.txt for new languages as well (it, ka, and pt_BR).

Yep, going to do that, I was just doing the Crowdin part before.

To the point that makes me hesitate merging this, as it would nuke credits to past contributors, let alone some useful comments...

To be honest putting those in the PO files was a good idea in the first place, as both PO and POT files are supposed to be generated/maintained by tooling and comments are not supposed to be preserved.

It'll complicate having that and using Crowdin at the same time, at least not without having a custom merge step after the download from Crowdin. The direct downloads from Crowdin won't work as-is.

Let me try doing a manual PO merge to see if I can preserve those.

P.S.: Note that the exact same workflow was done in the other PR for the 0.28 branch. I suppose that the comments were not important in that case, or that nobody noticed?

@kmilos
Copy link
Collaborator

kmilos commented Jul 10, 2024

P.S.: Note that the exact same workflow was done in the other PR for the 0.28 branch. I suppose that the comments were not important in that case, or that nobody noticed?

Both. I guess it was not a huge deal as we still have that historical information on main. Not sure we want to completely loose it though...

comments are not supposed to be preserved

Not sure about this either. Some tools use/show comments as hints to other translators/collaborators IIRC...

Let me try doing a manual PO merge to see if I can preserve those.

I was not happy w/ my attempt, but I hope there is a way to do this...

@lbschenkel
Copy link
Collaborator Author

lbschenkel commented Jul 10, 2024

omments are not supposed to be preserved

Not sure about this either. Some tools use/show comments as hints to other translators/collaborators IIRC...

Yes, but those comments should be in the source code near the translatable string, and xgettext will pick them up and include them in the .pot file. Those will be visible in Crowdin for translators.

The problem is that any ad-hoc comments put directly in the .po files are going to be overwritten when we regenerate those in Crowdin. Ad-hoc comment changes in .po files are better avoided for that reason — no matter using Crowdin or not, it just avoids any problems with the tooling.

When using Crowdin specifically, it's better to avoid any changes to .po files period. The default workflow when using Crowdin with gettext is that you upload .pot files and translate them via Crowdin web interface, and download .po files. You're no longer supposed to be changing .po files directly.

There are ways around that, but I'm explaining the simplest workflow that would require the least amount of work.

@lbschenkel lbschenkel force-pushed the master-i18n branch 2 times, most recently from f014849 to 4e517de Compare July 10, 2024 09:18
@lbschenkel
Copy link
Collaborator Author

Tried once more, now I merged the files with msgmerge instead of taking the PO files as-is from Crowdin. I think the relevant comments were preserved, but let me know if there's still something missing and I'll try harder.

I also sorted the files by message, so future updates will be much easier to diff.

I can also commit a shell script that helps with this, it's at least one step towards automating more.

- update exiv2.pot ('update-pot' build target)
- sync translations with Crowdin
- sorted files to make them more diffable (helps future PRs)
@lbschenkel
Copy link
Collaborator Author

Both. I guess it was not a huge deal as we still have that historical information on main. Not sure we want to completely loose it though...

When we're happy with this PR I can create a new one in that branch that'll restore those comments.

@kmilos
Copy link
Collaborator

kmilos commented Jul 10, 2024

Thanks, I'll have a closer look at the new po files when I get a chance to pull your branch, doesn't let me do it through the web UI...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L10n Translation languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants