-
Notifications
You must be signed in to change notification settings - Fork 18
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
Rebrand Firefox Account in english locales #25
Conversation
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.
All changes to existing strings require a new string ID.
Also, please only update en
, the other variants will be fixed in Pontoon by each locale.
en/messages.json
Outdated
"firefoxAccountsSync": { | ||
"message": "Firefox accounts sync:" | ||
"sync": { | ||
"message": "Data Synchronisation:" |
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.
This ain't American English? Same later ;-)
By curiosity, is this always the case? Also, is there some good practices to update it without writing a completely different id (e.g. my-string-id-v2)? |
See this doc for more details (adding a number to the previous ID works) It's not black and white, but in most cases (like 99%) you want a new ID. |
51c6669
to
46495ab
Compare
en/messages.json
Outdated
"firefoxAccountsSync": { | ||
"message": "Firefox accounts sync:" | ||
"sync": { | ||
"message": "Sync:" |
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.
Is there a specific reason to not use Synchronization
?
We can use Sync here, but that's just short for Synchronization when the space is limited, is not a feature anymore
https://acorn.firefox.com/latest/content/word-list/s-t-D8DB7Wjd
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.
I wasn't sure if we also changed the way to refer to Sync (as in Firefox Sync). So do you think using "Synchronization" for the title and "Enable synchronatization" for the permission string would be better in this case?
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.
I think it would make sense (look out for typos, Enable synchronization
).
46495ab
to
a0691ec
Compare
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.
LGTM. Let me know if it's ready to merge or if I should wait.
It's ready to be merged because we'd like to give time to the translators to update their locales before we push a new release of MAC :). |
Change strings containing Firefox Account to use a generic term "account" instead. I modified the 3 english locales and we'd like to land this patch before pushing a new version on AMO so contributors have time to update other locales.