Closed Bug 1073893 Opened 6 years ago Closed 5 years ago

Clean up MozL10n API use in System

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

x86
All
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(1 file)

Review use cases of:
 - mozL10n.translate
 - mozL10n.localize
 - mozL10n.get
 - LazyL10n
Assignee: nobody → gandalf
Blocks: 1020137, 1020138
Status: NEW → ASSIGNED
Attached file pull request
Hey, Kevin, I saw you're a peer of the module so I thought I'll ask you to review it since we can iterate over this in the same timezone. :)
Attachment #8500733 - Flags: review?(kgrandon)
Comment on attachment 8500733 [details] [review]
pull request

This breaks some basic Fx Accounts flows. Error messages for example are displayed with "Error message", instead of something like, "Unable to Connect". 

I've left a comment as to where I think this might be happening in the pull request. I haven't checked any other flows yet, but please re-flag me once this is addressed.
Attachment #8500733 - Flags: review?(kgrandon) → review-
Comment on attachment 8500733 [details] [review]
pull request

I fixed the issue and the tests are green (minus the perma-red Gij).

Rerequesting review.
Attachment #8500733 - Flags: review- → review?(kgrandon)
Comment on attachment 8500733 [details] [review]
pull request

I tried testing as much as I could and everything looks good to me. Thank you!
Attachment #8500733 - Flags: review?(kgrandon) → review+
Gandalf, shall we consider updating the entity name for strings here? https://github.com/zbraniecki/gaia/commit/52ee3c33aacd17ba32de3e187ecfc554ef127993#diff-27

Because it results in text displayed in statusbar for locales, until they catch up with this change :/
Flags: needinfo?(gandalf)
(In reply to Théo Chevalier [:tchevalier] from comment #6)
> Gandalf, shall we consider updating the entity name for strings here?
> https://github.com/zbraniecki/gaia/commit/
> 52ee3c33aacd17ba32de3e187ecfc554ef127993#diff-27
> 
> Because it results in text displayed in statusbar for locales, until they
> catch up with this change :/

I would hope we don't need to. We're safe for releases - no one will get a signoff with bad entities - we should just ask localizers to update master when they can. I'd like to be conservative with ID inflation unless our systems don't allow us to catch that. In this case they do, so I'd prefer to just let localizers catch up.
Flags: needinfo?(gandalf)
Alright, sounds good. Adding Delphine, so that she's aware those strings must be localized for sign-off :)
Depends on: 1088824
You need to log in before you can comment on or make changes to this bug.