1235095 - Do a first sync loop-client-l10n with github repo

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Mark Banner (:standard8)]

Now we've moved to github, we need to start to sync loop-client-l10n with the new repo. We probably want to do this fairly son so that we can get l10n started on the new strings.

See the user story for more details.

Comment 1

[Mark Banner (:standard8)]

Ed, would you be able to take this on next week when I think you're about?

Comment 2

[Ed Lee :Mardak]

There's some differences in unused strings:

https://github.com/mozilla/loop-client-l10n/blob/master/l10n/af/loop.properties#L23
call_with_contact_title=Conversation with {{incomingCallIdentity}}

https://github.com/mozilla/loop/blob/master/locale/en-US/add-on.properties#L118
call_with_contact_title=Conversation with {{contactName}}

I'll just take the existing string (incomingCallIdentity) for consistency of preferring "translated" strings.


Here's an example of split files:
> https://github.com/Mardak/loop-client-l10n/blob/4047650347bd27c1a26f92d8070124e351f36bd7/l10n/bs/add-on.properties
> https://github.com/Mardak/loop-client-l10n/blob/4047650347bd27c1a26f92d8070124e351f36bd7/l10n/bs/shared.properties
> https://github.com/Mardak/loop-client-l10n/blob/4047650347bd27c1a26f92d8070124e351f36bd7/l10n/bs/standalone.properties


For all the changes (ignore the xxx script) except fixing up template directory:
https://github.com/Mardak/loop-client-l10n/compare/master...bug-1235095-split

Comment 3

[Ed Lee :Mardak]

There's a few additional en_US loop.properties strings:

> active_screenshare_button_title=Stop sharing
> inactive_screenshare_button_title=Share your screen
> rooms_new_room_button_label=Start a conversation
> context_inroom_label2=Let's Talk About:

But other locales, e.g., bs have a lot more extra strings:

> missing_conversation_info=Nedostaju informacije o razgovoru.
> call_failed_title=Poziv neuspješan.
> generic_failure_title=Nešto je pošlo po zlu.
> generic_failure_with_reason2=Možete pokušati ponovo ili emailom poslati link da vam se jave kasnije.
> mute_local_video_button_title=Isključite zvuk vašeg videa
> unmute_local_video_button_title=Uključite zvuk vašeg videa
> active_screenshare_button_title=Zaustavi dijeljenje
> inactive_screenshare_button_title=Podijelite vaš ekran
> powered_by_webrtc=Audio i video komponente {{clientShortname}}-a pokreće WebRTC
> use_latest_firefox=Molimo da pokušate ovaj link u WebRTC-omogućenom browseru, poput {{firefoxBrandNameLink}}.
> call_url_unavailable_notification_heading=Ups!
> call_url_unavailable_notification_message2=Oprostite, ovaj URL nije dostupan. Možda je istekao ili pogrešno unešen.
> initiate_call_cancel_button=Otkaži
> invite_header_text=Pozovite nekoga da vam se pridruži.
> client_alttext={{clientShortname}} logo
> vendor_alttext={{clientShortname}} logo
> call_url_creation_date_label=(od {{call_url_creation_date}})
> call_progress_getting_media_title=Čekam na medij…
> call_progress_connecting_description=Povezujem…
> call_progress_ringing_description=Zvoni…
> fxos_app_needed=Molimo da instalirate {{fxosAppName}} aplikaciju sa Firefox Marketplace-a.
> first_time_experience_title={{clientShortname}} — Pridruži se razgovoru
> first_time_experience_button_label=Započnite
> rooms_list_copy_url_tooltip=Kopiraj link
> rooms_list_delete_tooltip=Obriši razgovor
> rooms_list_deleteConfirmation_label=Da li ste sigurni?
> rooms_new_room_button_label=Započni razgovor
> rooms_only_occupant_label=Vi ste prvi ovdje
> standalone_title_with_status={{clientShortname}} — {{currentStatus}}
> status_in_conversation=U razgovoru
> status_conversation_ended=Razgovor završen
> support_link=Dobijte pomoć
> context_inroom_label=Pričajmo o:

Comment 4

[Ed Lee :Mardak]

(Commenting on User Story)
> -- Note that l10n/templates and l10n/en-US should be the same in the
> loop-client-l10n repo.
Should loop-client-l10n/l10n/en_US/* match loop/locale/en-US/* ? The top of comment 3 shows that loop-client-l10n repository would have 4 extra strings that loop repository doesn't have.

Comment 5

[Ed Lee :Mardak]

Also noted on irc:

lv has a string using the wrong token, so "Izmantojot Firefox Hello, jūs piekrītat {{terms_of_use}}..." is left unreplaced instead of {{terms_of_use_url}}

en_GB has a double-escaped backslash and shows up as: "Self-view hidden but still being sent; resize window \\n to show"

Comment 6

[Ed Lee :Mardak]

Also I left a blank localization note comment above the strings that existed in loop.properties but not used in the loop repository, e.g., https://github.com/Mardak/loop-client-l10n/blob/4047650347bd27c1a26f92d8070124e351f36bd7/l10n/bs/shared.properties#L45

This means some strings are moved to the end of shared.properties file from their original loop.properties location. Is that okay (assuming I come up with a reasonable localization note? or maybe the note isn't necessary).

Comment 7

[Matjaz Horvat [:mathjazz]]

(Commenting on User Story)
> -- Note that l10n/templates and l10n/en-US should be the same in the
> loop-client-l10n repo.

Just curious - can we drop loop-client-l10n/en-US to avoid redundancy?

(In reply to Ed Lee :Mardak from comment #3)
> But other locales, e.g., bs have a lot more extra strings:

That means the Bosnian file is out of date (indeed, last change was made 3 months ago). You can completely remove obsolete strings from locale files that are no longer present in templates.

:Mardak, when you're done, you can make a PR in loop-client-l10n and r? me (mathjazz) and flod. Once we are ready on our side of things, we'll r+.

Comment 8

[Mark Banner (:standard8)]

(In reply to Ed Lee :Mardak from comment #3)
> There's a few additional en_US loop.properties strings:

These we should can just drop. They're old strings we used to need but now don't.

> But other locales, e.g., bs have a lot more extra strings:

At a glance, these all look like old strings - I'm guessing the locale hasn't updated recently, so if we haven't got them in our locale files, we should just drop them.

(In reply to Ed Lee :Mardak from comment #4)
> (Commenting on User Story)
> > -- Note that l10n/templates and l10n/en-US should be the same in the
> > loop-client-l10n repo.
> Should loop-client-l10n/l10n/en_US/* match loop/locale/en-US/* ? The top of
> comment 3 shows that loop-client-l10n repository would have 4 extra strings
> that loop repository doesn't have.

It probably used to last time we did a merge/release. However, with this update we can make the l10n repo match our new files.

(In reply to Ed Lee :Mardak from comment #5)
> Also noted on irc:
> 
> lv has a string using the wrong token, so "Izmantojot Firefox Hello, jūs
> piekrītat {{terms_of_use}}..." is left unreplaced instead of
> {{terms_of_use_url}}
> 
> en_GB has a double-escaped backslash and shows up as: "Self-view hidden but
> still being sent; resize window \\n to show"

It'd be good to file a follow-up to investigate these errors. We should get these fixed, but we perhaps want some sort of automation or something to keep an eye (l10n may already have stuff with their newer code, but I'd need ask around a bit).

Comment 9

[Mark Banner (:standard8)]

(In reply to Matjaz Horvat [:mathjazz] from comment #7)
> (Commenting on User Story)
> > -- Note that l10n/templates and l10n/en-US should be the same in the
> > loop-client-l10n repo.
> 
> Just curious - can we drop loop-client-l10n/en-US to avoid redundancy?

I thought that was required for verbatim/pontoon. However, if we can only have the templates directory in the l10n repo, that would probably be useful.

Comment 10

[Francesco Lodolo [:flod]]

Webstatus currently uses the en_US folder as reference, I need to fix it (and that requires code changes on my side).

Comment 11

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #10)
> Webstatus currently uses the en_US folder as reference, I need to fix it
> (and that requires code changes on my side).

Code updated, it's already checking against templates
https://l10n.mozilla-community.org/~flod/webstatus/?product=loop-client-l10n

Still need to work on the hard-coded list of files, but eventually I can work around that by adding the new files to the list.

Comment 12

[Ed Lee :Mardak]

Attachment: Link to Github pull-request: https://github.com/mozilla/loop-client-l10n/pull/7

Removed l10n/en_US and split out loop.properties into 3 files: add-on/shared/standalone.properties using existing strings then en-US strings as backup. Strings not existing in the loop repository are removed.

Comment 13

[Francesco Lodolo [:flod]]

Comment on attachment 8704026 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop-client-l10n/pull/7

Unfortunately the diff is impossible to view on GitHub, so I checked the Italian files in your branch. Based on that the patch looks good to me, not sure if there's a better way to check.

Can you share the script you used to create this PR? You might have to deal with a rebase (I fixed an error in Pontoon noticed while checking the file).

Comment 14

[Ed Lee :Mardak]

(In reply to Francesco Lodolo [:flod] from comment #13)
> Can you share the script you used to create this PR?
https://github.com/Mardak/loop-client-l10n/commit/79dce72585ca588df4c2bba13ee0bc7b0c4da916

> You might have to deal with a rebase (I fixed an error in Pontoon noticed while checking the file).
Rebased to include the legal_text_and_links=Utilizzando change.
https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split/l10n/it/standalone.properties#L20

Comment 15

[Francesco Lodolo [:flod]]

(In reply to Ed Lee :Mardak from comment #14)
> https://github.com/Mardak/loop-client-l10n/commit/
> 79dce72585ca588df4c2bba13ee0bc7b0c4da916

Thanks, I can't spot anything wrong there either.

Comment 16

[Matjaz Horvat [:mathjazz]]

(In reply to Ed Lee :Mardak from comment #12)
> Removed l10n/en_US and split out loop.properties into 3 files:
> add-on/shared/standalone.properties using existing strings then en-US
> strings as backup.

If a string is not translated, it should not be included in the locale file, because it's impossible to tell if it's untranslated or identical.

Comment 17

[Francesco Lodolo [:flod]]

(In reply to Matjaz Horvat [:mathjazz] from comment #16)
> If a string is not translated, it should not be included in the locale file,
> because it's impossible to tell if it's untranslated or identical.

Uh, I assumed Pontoon managed to recognize a brand new string coming from VCS (how did you deal with moving the project from Verbatim?).

Comment 18

[Matjaz Horvat [:mathjazz]]

(In reply to Francesco Lodolo [:flod] from comment #17)
> Uh, I assumed Pontoon managed to recognize a brand new string coming from
> VCS (how did you deal with moving the project from Verbatim?).

I remember removing strings for some locales.

Verbatim required English strings in locale files, but that approach doesn't work (as explained above), so localizers were having problems with strings like "Firefox" or "Mozilla" appearing as untranslated.

Comment 19

[Ed Lee :Mardak]

(In reply to Matjaz Horvat [:mathjazz] from comment #16)
> (In reply to Ed Lee :Mardak from comment #12)
> If a string is not translated, it should not be included in the locale file
Should I have the string key and an empty string for untranslated strings? E.g.,

rooms_server_unavailable_message=

This would allow the split file structure to be maintained with ## LOCALIZATION NOTEs -- otherwise files like it/add-on.properties would be completely empty:

https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split/l10n/it/add-on.properties

Comment 20

[Matjaz Horvat [:mathjazz]]

(In reply to Ed Lee :Mardak from comment #19)
> otherwise files like it/add-on.properties would be completely empty:

That's actually OK. It follows the pattern used by other projects with .properties file format like Firefox or Firefox OS. Normally we don't even include empty files in locale repositories.

Comment 21

[Ed Lee :Mardak]

Here's an example of only including the existing "translated" strings:

https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split-2/l10n/it/add-on.properties

Most of it is empty relative to the amounts of LOCALIZATION NOTEs copied over. Also, there's some existing strings that weren't translated but got moved over from loop.properties, e.g., 

legal_text_tos = Terms of Use

Here's the other 2 split out files:
https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split-2/l10n/it/shared.properties
https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split-2/l10n/it/standalone.properties

Comment 22

[Ed Lee :Mardak]

(In reply to Ed Lee :Mardak from comment #21)
> Here's an example of only including the existing "translated" strings:
Sorry I misspoke. Fixing an issue with the script. (legal_text_tos has spaces around "=")

Comment 23

[Matjaz Horvat [:mathjazz]]

(In reply to Ed Lee :Mardak from comment #21)
> Here's an example of only including the existing "translated" strings:
> 
> https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split-2/l10n/it/
> add-on.properties
> 
> Most of it is empty relative to the amounts of LOCALIZATION NOTEs copied
> over.

Yeah, it's hard to be smart about comments in .properties file, I'm fine with leaving them in like this.

> Also, there's some existing strings that weren't translated but got
> moved over from loop.properties, e.g., 
> 
> legal_text_tos = Terms of Use

Where do these strings come from? Can't find them in the old file:
https://github.com/mozilla/loop-client-l10n/blob/master/l10n/it/loop.properties

> Here's the other 2 split out files:
> https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split-2/l10n/it/
> shared.properties
> https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split-2/l10n/it/
> standalone.properties

Both of these look great, nice work!

Comment 24

[Ed Lee :Mardak]

(In reply to Matjaz Horvat [:mathjazz] from comment #23)
> Where do these strings come from?
These come from https://github.com/mozilla/loop/blob/master/locale/en-US/add-on.properties#L154

Here's a better example of existing "translated" strings:
https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split-2/l10n/af/add-on.properties#L59

from:
https://github.com/mozilla/loop-client-l10n/blob/master/l10n/af/loop.properties#L40

I've updated the pull request. https://github.com/mozilla/loop-client-l10n/pull/7

Comment 25

[Matjaz Horvat [:mathjazz]]

(In reply to Ed Lee :Mardak from comment #24)
> These come from
> https://github.com/mozilla/loop/blob/master/locale/en-US/add-on.
> properties#L154

So these are the newly exposed strings to be translated, right? In this case, they should be only merged to the source files (in the templates folder).

> Here's a better example of existing "translated" strings:
> https://github.com/Mardak/loop-client-l10n/blob/bug-1235095-split-2/l10n/af/
> add-on.properties#L59
> 
> from:
> https://github.com/mozilla/loop-client-l10n/blob/master/l10n/af/loop.
> properties#L40

Yup, that's a relic from the past. We should probably address this in a separate bug. Looking at https://l10n.mozilla-community.org/~flod/webstatus/?product=loop-client-l10n at least 25 locales need to be cleaned.

Comment 26

[Ed Lee :Mardak]

(In reply to Matjaz Horvat [:mathjazz] from comment #25)
> So these are the newly exposed strings to be translated, right? In this
> case, they should be only merged to the source files (in the templates
> folder).
Yup. That's how the updated pull request looks now. Just confirming, en_US should be removed and only templates has the strings to be translated.

> Yup, that's a relic from the past. We should probably address this in a separate bug.
Okay.

The pull request should be good to go unless we want to clean up some of the templates strings. I noticed duplicate strings that should probably be in shared.properties instead:

https://github.com/mozilla/loop/blob/master/locale/en-US/add-on.properties#L112
https://github.com/mozilla/loop/blob/master/locale/en-US/standalone.properties#L23

Comment 27

[Matjaz Horvat [:mathjazz]]

This is good to go from my perspective as well.

(unless those duplicate strings are redundant)

Comment 28

[Ed Lee :Mardak]

https://github.com/mozilla/loop-client-l10n/commit/b1ca32d4bc1207f39ed59fefdf26a47b9702f97a