936889 - 929552 Followup - Sync string for exported contacts

Status: VERIFIED FIXED

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

Description

[Fernando Campo (:fcampo)]

In bug 929552 we changed the strings for the result of exporting contacts, but we use the word contact when we don't export any, and friend when we export some.

We should use the same term for all.

Comment 1

[Fernando Campo (:fcampo)]

Attachment: Link to PR - https://github.com/mozilla-b2g/gaia/pull/13550

Comment 2

[Francesco Lodolo [:flod]]

Comment on attachment 829849 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/13550

Not sure I should be the one reviewing this. 

Anyhow, at this point you need to change IDs, because strings already landed on master and we need to be sure localizers pick up the update (for example numContactsExported).

Comment 3

[Fernando Campo (:fcampo)]

Comment on attachment 829849 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/13550

Sorry for the delay, got a few days off.

Changing reviewer as per Francesco's comment 2, please reassign if needed.
Updated the pull request with IDs changed.

Comment 4

[Axel Hecht [:Pike]]

Comment on attachment 829849 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/13550

This is really a code change, and should be reviewed by the gaia team.

Forwarding this request to Francisco, as he reviewed the original patch.

Shouldn't have done an r+ on the original patch myself, tsktsktsk, bad bunny.

Comment 5

[Fernando Campo (:fcampo)]

Failing tests are not related, so merged on master a9f87d4c5ee467026b165e2f1f59b0ec03418028

Thanks for the review, and sorry for pushing the first wrong code :(

Comment 6

[Axel Hecht [:Pike]]

Sorry, but this patch is broken. The plural form needs {[ ]} and not {{}}.

Patch reading is hard.

https://developer.mozilla.org/en-US/Firefox_OS/Localizing_Firefox_OS_Apps#Plural_macro has docs.

Comment 7

[Fernando Campo (:fcampo)]

I seriously don't know what's wrong with me :( 3 simple patches, all with stupid mistakes, that's careless.

Reverted commit on master 0ec3a39cb8ba4bd01faad455ad6b6793bd06ac4e

Patch updated, please check it Pike.

Comment 8

[Axel Hecht [:Pike]]

And I was involved in all three of them. Time for vacation.

The backout looks good, I guess. I don't see the update in the PR yet?

Comment 9

[Fernando Campo (:fcampo)]

Attachment: Updated patch - https://github.com/mozilla-b2g/gaia/pull/13955

Seriously, don't know what's wrong with me. Really careless patches, my apologies.

Thanks for catching it, Pike. Could you take a look at the update?
Cheers.

Comment 10

[Axel Hecht [:Pike]]

Comment on attachment 8336817 [details] [review]
Updated patch - https://github.com/mozilla-b2g/gaia/pull/13955

We're doing reviews to catch these errors before they land, so it's as much my bad as it is yours.

I did this review with kdiff3 on a local clone, which is much better at showing in-line changes, and this should work. r=me, knocking on wood.

Comment 11

[Fernando Campo (:fcampo)]

on master - https://github.com/mozilla-b2g/gaia/commit/10e5cd63155a41927b9fa3b5a2bd0726ed85f1ab

I should start to pay a fine for every wrong commit :p

Comment 12

[Jason Smith [:jsmith]]

koi+ for the same reasons as https://bugzilla.mozilla.org/show_bug.cgi?id=929552#c18

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 10e5cd63155a41927b9fa3b5a2bd0726ed85f1ab
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 14

[Axel Hecht [:Pike]]

We'll need this fix on top of bug 936889. Did you try to apply that bug before this one?

Comment 15

[Fernando Campo (:fcampo)]

I solved it locally, but again, tree closed, so I'm unable to push the commit to the branch. No joy before christmas.

Comment 17

[Axel Hecht [:Pike]]

(In reply to Fernando Campo (:fcampo) from comment #15)
> I solved it locally, but again, tree closed, so I'm unable to push the
> commit to the branch. No joy before christmas.

Preeti, can we can get exception for this landing?

This is fixing up strings that are still waiting to go out on 1.2. We're already deep in the woods for those strings, given that we're supposed to be done with 1.2 yesterday, and waiting for a while with this hurts by the day.

Comment 18

[John Ford [:jhford] CET/CEST Berlin Time]

This bug was partially uplifted.

Uplifted 10e5cd63155a41927b9fa3b5a2bd0726ed85f1ab to:
v1.3 already had this commit

Commit 10e5cd63155a41927b9fa3b5a2bd0726ed85f1ab didn't uplift to branch v1.2

Comment 19

[Axel Hecht [:Pike]]

https://github.com/mozilla-b2g/gaia/blob/v1.2/apps/communications/contacts/locales/contacts.en-US.properties#L107 doesn't have this fix.

We *really* need to get this landed.

Comment 20

[Fernando Campo (:fcampo)]

ok, now that the tree is open again I'm uplifting it to v1.2

Comment 21

[Fernando Campo (:fcampo)]

pushed to v1.2 - fcf1c2fe020c29da4755621cbffdc1a333a43be9

Comment 22

[isabelrios]

Verfied in both v1.3 12/16 build:
Gecko-6e22321
Gaia-1752e9e

And v1.2 12/16 build:
Gecko-f6f325b
Gaia-fcf1c2f

Now it appears x/y exported