Closed Bug 1176774 Opened 6 years ago Closed 6 years ago

Upgrade most of Loop's libraries to the latest versions


(Hello (Loop) :: Client, defect)

Not set


(firefox42 fixed)

42.1 - Jul 13
Tracking Status
firefox42 --- fixed


(Reporter: standard8, Assigned: standard8)




(2 files)

Its about time we upgraded to the latest versions of the libraries we use to keep pace with features and fixes etc.

I've got a couple of patches to bring these up to date. I'm keeping React & eslint separate as they will be more complicated to update.
Flags: qe-verify-
Flags: firefox-backlog+
Simple upgrade of the test libraries.
Attachment #8625376 - Flags: review?(mdeboer)
This upgrades most of our main libraries.

I've included minified versions of backbone & lodash. Its unclear if it'll help desktop perf (react "prod" is an improvement on the dev one, as they don't do extra desktop checks), but getting minified versions of these into the tree for when we sort out loop-client seems like a good idea.

I had to skip one test as I think Backbone has changed how it does things, and as a result the test fails. However, closer inspection of the test reveals that its currently testing the wrong thing, and that it seems the test is working currently by side-effects. Given that, plus the test is in the code which we're hopefully going to dump as soon as we get rid of the call-url stuff - then I think skipping is reasonable.
Attachment #8625377 - Flags: review?(mdeboer)
Comment on attachment 8625376 [details] [diff] [review]
Upgrade Loop's test libraries.

Review of attachment 8625376 [details] [diff] [review]:

Don't mind who reviews this first.
Attachment #8625376 - Flags: review?(dmose)
Attachment #8625377 - Flags: review?(dmose)
Blocks: 1178270
Comment on attachment 8625376 [details] [diff] [review]
Upgrade Loop's test libraries.

Review of attachment 8625376 [details] [diff] [review]:

Looks good; r=dmose.

Please post to loop-services-dev or some other appropriate place once this has landed, with the notes to the release notes, and be sure to call out the one change that people are likely to care about: the fact that sinon now makes it possible to stub getters and setters with a documentation link to how to do that.
Attachment #8625376 - Flags: review?(mdeboer)
Attachment #8625376 - Flags: review?(dmose)
Attachment #8625376 - Flags: review+
Comment on attachment 8625377 [details] [diff] [review]
Upgrade Loop's libs to latest versions and add some more pre-minified ones for release.

Review of attachment 8625377 [details] [diff] [review]:

I'm a bit concerned about switching the desktop stuff to the minified versions:

* it's not clear that it'll make a performance difference, given that there's no network load, and this stuff is all compressed inside a JAR anyway
* even if the source-mapping stuff works, the console doesn't currently support source maps (though I think many of the other devtools do)

meaning that it might make debugging production nightly builds a bit more painful.

That said, I'm not too worried about this, so I'll leave it up to you.

It also looks like this patch will need a rebase before landing.

Please post to loop-services-dev with links to the changelog once this patch lands.

::: browser/components/loop/test/shared/views_test.js
@@ +460,5 @@
>        });
>        describe("#startPublishing", function() {
>          beforeEach(function() {
> +          sandbox.stub(fakePublisher, "listenTo");

Is this the right change, now that the test has been removed?

@@ +471,5 @@
>            sinon.assert.calledOnce(fakeSession.publish);
>          });
> +        // XXX This test would need reworking, but the code should be going
> +        // away soon.

Instead of soon, how about either "as part of" or "after" bug XXX (the call-URL removal bug) lands?
Attachment #8625377 - Flags: review?(mdeboer)
Attachment #8625377 - Flags: review?(dmose)
Attachment #8625377 - Flags: review+
Keywords: leave-open
Iteration: 41.3 - Jun 29 → 42.1 - Jul 13
(In reply to Pulsebot from comment #9)

Landed this second patch without the minified for production as in my testing, we were using about 200k more of memory.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.