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

fix(server.po): recover email string translations #36

Merged
merged 1 commit into from Feb 26, 2015

Conversation

zaach
Copy link
Contributor

@zaach zaach commented Feb 25, 2015

@shane-tomlinson @mathjazz I've added the translations back and updated the template.

@mathjazz
Copy link
Contributor

Looks good. Just a quick question. The following pairs are almost identical:

Reset password - Reset password:
Verify account - Verify account:
Verify - Verify:

Are they intentionally different or can we unify them?

@shane-tomlinson
Copy link
Contributor

Looks good. Just a quick question. The following pairs are almost identical:

Reset password - Reset password:
Verify account - Verify account:
Verify - Verify:

Are they intentionally different or can we unify them?

@ryanfeeley - what do you think about some unification? "Verify account" and "Verify" seem reasonable.

@mathjazz - the colon is to separate the text from a verification link that follows. What is the best practice there?

@mathjazz
Copy link
Contributor

OK, in this case "string - string:" pairs should be separate strings, as they could be different parts of speech (verb vs. noun).

@shane-tomlinson
Copy link
Contributor

LGTM! @jrgm are these the strings that will be used for today's prod push?

@zaach
Copy link
Contributor Author

zaach commented Feb 25, 2015

The prod push is using an earlier hash– this will just affect deployments using fxa-dev.

@mathjazz
Copy link
Contributor

That means some strings will be left untranslated after the new release or is this already the case in prod?

@zaach
Copy link
Contributor Author

zaach commented Feb 25, 2015

@mathjazz We're using this hash: 9b283e2, which has the email strings but won't have translations to strings made in the last 5 days. But that's usually the case.

@mathjazz
Copy link
Contributor

OK. To make "that's usually the case" clear - we usually push to production first and give strings to localizers after that?

@zaach
Copy link
Contributor Author

zaach commented Feb 25, 2015

@mathjazz Yeah, actually I merge strings for train 32 earlier than necessary. We could wait and merge them after the current train is pushed to production. As it stands, localizers will have extra time for train 32.

@mathjazz
Copy link
Contributor

Okay, thanks for explaining this @zaach!

@jrgm
Copy link
Contributor

jrgm commented Feb 25, 2015

Slightly off-topic, but about L10N and production releases, I wrote this micro-note on how the rpm is built with a specific version of fxa-content-server-l10n (default is HEAD). https://github.com/mozilla-services/svcops/blob/master/services/firefox-accounts/fxa-content-server/README.md

@shane-tomlinson
Copy link
Contributor

@zaach - it looks like this needs to be rebased or done again. A few commits came in after this PR was created and a clean merge is no longer possible.

@zaach
Copy link
Contributor Author

zaach commented Feb 26, 2015

Rebased.

@shane-tomlinson
Copy link
Contributor

Thanks @zaach, @mathjazz, what do you think, can we hit "merge"?

@mathjazz
Copy link
Contributor

Let's hit the merge!

@zaach
Copy link
Contributor Author

zaach commented Feb 26, 2015

Merging 👍

zaach added a commit that referenced this pull request Feb 26, 2015
fix(server.po): recover email string translations
@zaach zaach merged commit 6d47ec3 into master Feb 26, 2015
@zaach zaach deleted the recover-email-templates branch February 26, 2015 19:22
@mathjazz
Copy link
Contributor

Verbatim updated.

But -- it seems like some locales aren't in sync with the templates:
https://localize.mozilla.org/projects/accounts/

(Sort by "Total".)

@zaach zaach mentioned this pull request Feb 26, 2015
@zaach
Copy link
Contributor Author

zaach commented Feb 26, 2015

@mathjazz Thanks, I've opened another PR. Not sure how those didn't make it into the initial train 32 PR.

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.

None yet

4 participants