1236863 - Bring v2.5 code in apps/sync/ in line with master

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Sync, defect)

Description

[Michiel de Jong [:michielbdejong]]

Letting shared/js/sync in master diverge from the corresponding files in apps/sync in the v2.5 branch would mean splitting our attention between the diverged code bases, so let's keep them 100% identical.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] michielbdejong:1236863-sync-app-from-master > mozilla-b2g:v2.5

Comment 2

[Michiel de Jong [:michielbdejong]]

Comment on attachment 8704096 [details] [review]
[gaia] michielbdejong:1236863-sync-app-from-master > mozilla-b2g:v2.5

We have to see how we can run TreeHerder tests on v2.5, due to https://bugzilla.mozilla.org/show_bug.cgi?id=1235762#c6

Comment 3

[Michiel de Jong [:michielbdejong]]

I updated the PR (script use in comments, this already includes the patch from bug 1237570) and added a second commit to make the tests pass.

Comment 4

[Michiel de Jong [:michielbdejong]]

Comment on attachment 8704096 [details] [review]
[gaia] michielbdejong:1236863-sync-app-from-master > mozilla-b2g:v2.5

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 5

[Michiel de Jong [:michielbdejong]]

[Bug caused by] (feature/regressing bug #): This contains all the fixes we made to FxSync since v2.5 was branched. To avoid diverging code bases, we would like to update the sync app in its entirety.

[User impact] if declined: Main problem would be the sync feature would not work for users with > 1000 URLs in their browser history

[Testing completed]: We ran the unit tests from apps/sharedtest/test/unit on this branch, using node v0.12.5 and npm v2.4.1. The only failing test there is an unrelated one:

1) [sharedtest-test/unit/tag_visibility_monitor_test.js] tag_visibility_monitor basics:
     Uncaught Error: Error: expected undefined to deeply equal { Object (0, 1, ...) } (app://sharedtest.gaiamobile.org/common/test/helper.js?time=1452180828929:32)
      at onerror (app://sharedtest.gaiamobile.org/common/vendor/mocha/mocha.js:4959:10)

We also tested manually using B2G-desktop from https://groups.google.com/d/msg/mozilla.dev.fxos.sync/LjpG6KwXp0c/us1IHf83BwAJ

[Risk to taking this patch] (and alternatives if risky): if we made an error copying the files (see https://github.com/mozilla-b2g/gaia/pull/33744#issuecomment-169672240) or if there was a change in apps/system that was not made in tv_apps/smart-system that we didn't think of, that could cause a problem, but we're not aware of any such risk. The code in this patch is already live for phone Foxfooders on the nightly channel.

[String changes made]: None

Comment 6

[Josh Cheng [:josh]]

Comment on attachment 8704096 [details] [review]
[gaia] michielbdejong:1236863-sync-app-from-master > mozilla-b2g:v2.5

Approved for TV 2.5

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Thank you Josh. Michiel, could you add 'a=josh' into the commit message, please?

Comment 8

[Michiel de Jong [:michielbdejong]]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #7)
> Thank you Josh. Michiel, could you add 'a=josh' into the commit message,
> please?

done

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

https://github.com/mozilla-b2g/gaia/commit/5af3f58c84caeac2091ec9e79e39aecfb41198ba

Comment 10

[Carsten Book [:Tomcat]]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #9)
> https://github.com/mozilla-b2g/gaia/commit/
> 5af3f58c84caeac2091ec9e79e39aecfb41198ba

and so 2.5 is fixed :)