Closed Bug 1262389 Opened 4 years ago Closed 4 years ago

Upgrade moz-kinto-client to kinto.js 2.0

Categories

(Cloud Services :: Firefox: Common, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: leplatrem, Assigned: leplatrem)

References

Details

Attachments

(1 obsolete file)

The current code landed in Bug 1244776 (https://hg.mozilla.org/mozilla-central/rev/75fe50edcf9a) was taken from kinto.js master branch (unknown commit).

First we should tag a new version of Kinto.js (2.0 because of transaction breaking changes, already landed by mark).

And then, we should land a tagged version instead of some commit from master branch. 

This would also have the benefit to introduce the necessary client features (eg. hooks) for collection signing (Bug 1198614)
* Land kinto.js 2.0, with transactions, indexeddb optimizations and incoming-changes hooks for content signing
* Renamed moz-kinto-client.js to kinto-offline-client.js
* Introduced kinto-http-client.js for direct interaction with Kinto HTTP API

Review commit: https://reviewboard.mozilla.org/r/45447/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45447/
Attachment #8739941 - Flags: review?(mgoodwin)
Attachment #8739941 - Flags: review?(MattN+bmo)
FFIW, obvious r+ from me as all this has been pair programmed with Mathieu.
See Also: → 1250191
(In reply to Mathieu Leplatre (:leplatrem) from comment #0)
> The current code landed in Bug 1244776
> (https://hg.mozilla.org/mozilla-central/rev/75fe50edcf9a) was taken from
> kinto.js master branch (unknown commit).

FYI, this was commit 35532d60b7218709015b6da54bd0fae90d1c04c4
Blocks: 1263602
Comment on attachment 8739941 [details]
MozReview Request: Bug 1262389 - Update to kinto.js 2.0

https://reviewboard.mozilla.org/r/45447/#review42011

LGTM
Attachment #8739941 - Flags: review?(mgoodwin) → review+
Comment on attachment 8739941 [details]
MozReview Request: Bug 1262389 - Update to kinto.js 2.0

https://reviewboard.mozilla.org/r/45447/#review42889

Are you looking for me to do a thorough review of the changes? If so, seeing a diff from the previous for the renamed would be much easier.

::: services/common/KintoCertificateBlocklist.js:11
(Diff revision 1)
>  
>  this.EXPORTED_SYMBOLS = ["OneCRLClient"];
>  
>  const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
>  
> -Cu.import("resource://services-common/moz-kinto-client.js");
> +Cu.import("resource://services-common/kinto-offline-client.js");

If there is lots of code in common then this should be implemented as an `hg move` of the files which would also make it easier to review the changes and follow hg history.
https://reviewboard.mozilla.org/r/45447/#review42889

I don't think it is necessary. This library is developped elsewhere [0] and we land here the bundle generated with browserify/babel.

What is reassuring is that the update did not require any profond changes in the current code base (appart the renaming and mocking server calls in tests).

[0] https://github.com/Kinto/kinto.js
https://reviewboard.mozilla.org/r/45447/#review42889

> If there is lots of code in common then this should be implemented as an `hg move` of the files which would also make it easier to review the changes and follow hg history.

There was a lot of refactoring in the upstream code which might not be straightforward to review (mainly because of the split between the offline part and the http client).

And to be honest, I am not as confortable with hg as I would be with a ``git rebase`` to republish a nice history :(

Please tell me if this imprescidible and I will of course act accordingly :)
Comment on attachment 8739941 [details]
MozReview Request: Bug 1262389 - Update to kinto.js 2.0

https://reviewboard.mozilla.org/r/45447/#review43209

OK, I'll land now with an `hg move`
Attachment #8739941 - Flags: review+
Attachment #8739941 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/e687d8e0871d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.