916293 - [Clock]: Changes to nRemainHours need ID change to get picked up by l10n

Status: RESOLVED FIXED

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

Description

[Axel Hecht [:Pike]]

https://github.com/mozilla-b2g/gaia/commit/718c7638b05697b2d7f761f7462a62aa6ff7dc97 doesn't mention bug nor review, so it's hard to track down.

It did change the context in which nRemainHours is used, though, and this won't be picked up by l10n for 1.2.

Comment 1

[Corey Frang [:gnarf]]

Attachment: patch v1

Comment 2

[Corey Frang [:gnarf]]

Comment on attachment 809355 [details] [diff] [review]
patch v1

Axel - Could you please take a look at this and let me know if I interpreted the meaning of this bug correctly?  Thanks!

Comment 3

[Corey Frang [:gnarf]]

This seems to be a request from the l10n team for 1.2 - marking koi?

Comment 4

[Axel Hecht [:Pike]]

Comment on attachment 809355 [details] [diff] [review]
patch v1

Review of attachment 809355 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, this is the intention.

Comment 5

[Dylan Oliver [:doliver]]

blocking since this is being requested by l10n

Comment 6

[Mike Pennisi [:jugglinmike]]

Comment on attachment 809355 [details] [diff] [review]
patch v1

Review of attachment 809355 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me. I have some questions about process for the localization team, but (since they've already given this "feedback+") it shouldn't block this patch. I'll flag them for "needsinfo" in just a moment.

Comment 7

[Mike Pennisi [:jugglinmike]]

Does this bug require the "late-l10n" flag? Also, would you mind verifying/assigning bug 921093, which I have created to track the "staleness" of the test localization files hosted in the Gaia repository? Thanks!

Comment 8

[Axel Hecht [:Pike]]

This is a question for release management at which point they want to track and nail down late-l10n bugs.

PS: [e-mail] is an alias, and there's no real ownership on requests on that alias.

Comment 9

[Corey Frang [:gnarf]]

Attachment: patch v2

Same as the last patch only I added units to cover all of the render cases.

Comment 10

[Corey Frang [:gnarf]]

Comment on attachment 811108 [details] [diff] [review]
patch v2

Review of attachment 811108 [details] [diff] [review]:
-----------------------------------------------------------------

::: apps/clock/test/unit/banner_test.js
@@ +21,5 @@
> +
> +    // +10 hours +12 minutes + 20 seconds
> +    this.hours = this.minutes + (10 * 60 * 60 * 1000);
> +
> +    // +2 day +10 hours + 12 minutes + 20 seconds

In my local copy I've fixed all of the +x ____ to be + right before the number (no space)

@@ +41,5 @@
>    suite('#show', function() {
>      test('should make notification visible ', function(done) {
>        var visible;
>  
>        this.banner.show(this.fourish);

In my local copy I've also updated this.fourish to be this.hours

Comment 11

[Mike Pennisi [:jugglinmike]]

Comment on attachment 811108 [details] [diff] [review]
patch v2

Review of attachment 811108 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for adding these unit tests, Corey!

This should be good to land, provided you make the changes you described (plus the one I've requested).

::: apps/clock/test/unit/banner_test.js
@@ +54,5 @@
>    suite('#render', function() {
> +    setup(function() {
> +      this._ = this.sinon.spy(navigator.mozL10n, 'get');
> +    });
> +    test('minutes ahead', function(done) {

Since these tests are synchronous, we don't have to use the `done` callback. (I don't know why it was used in the previous test.)

Comment 12

[Corey Frang [:gnarf]]

Thanks for catching the `done` -- copy pasta gets me every time!

Comment 13

[Corey Frang [:gnarf]]

master: https://github.com/mozilla-b2g/gaia/commit/7c372b9919ae0de5843e2f842c0c99b7d628126d

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8f961547f698

Comment 15

[James Lal [:lightsofapollo]]

Uplifted 7c372b9919ae0de5843e2f842c0c99b7d628126d to:
v1.2: ad8b627e15819acf5526d6e213615ba3cffbb211