609421 - Combine various JSMs

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Philipp von Weitershausen [:philikon]]

* all type_record/* files can be merged with the relevant engines/* files
* the base_record/* files can be merged into one (top-level) file
* tracker.js and store.js can be merged into engines.js

Comment 1

[Richard Newman [:rnewman]]

Would be nice to wait for Bug 603489 before attempting this; otherwise it'd be a hell of a merge.

Comment 2

[Philipp von Weitershausen [:philikon]]

status.js can be merged into main.js.

(Rationale: While we want to keep main.js as lightweight as possible, as not to penalize users who don't use Sync, we don't have to worry about about status.js: anybody who wants to check whether Sync is configured will import main.js and call Weave.Status.checkSetup() which will import status.js anyway.)

Comment 3

[Philipp von Weitershausen [:philikon]]

auth.js can be merged into resource.js

Comment 4

[Philipp von Weitershausen [:philikon]]

Attachment: Part 1: Death to type_records [Checked in: See comment 19]

Combine type_record/* files with their respective engines/* counterparts

Comment 5

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2: Move store and tracker implementation into engines.js [Checked in: See comment 18]

Comment 6

[Philipp von Weitershausen [:philikon]]

Attachment: Part 3: Combine base_record/* files into record.js [Checked in: Seec omment 17]

Comment 7

[Philipp von Weitershausen [:philikon]]

Attachment: Part 4: Combine auth.js and resource.js [Checked in: See comment 15]

auth.js and resource.js are very much designed to work together, so let's combine them.

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 504619 [details] [diff] [review]
Part 1: Death to type_records
[Checked in: See comment 19]

Thanks for arranging these patches neatly :)

I'm going to review in layers, too. This one is good.

Comment 9

[Richard Newman [:rnewman]]

Comment on attachment 504626 [details] [diff] [review]
Part 3: Combine base_record/* files into record.js
[Checked in: Seec omment 17]

Looks good. 

Patch failed to apply on one of the tests, which is bizarre, because it's a trivial change.

Easy to fix by hand, and produces a patch of the same size, so I'm not concerned.

Comment 10

[Richard Newman [:rnewman]]

Comment on attachment 504629 [details] [diff] [review]
Part 4: Combine auth.js and resource.js
[Checked in: See comment 15]

As mentioned on IRC, these patches as presented need a little fiddling to apply, but you're probably all set in your queue.

Comment 11

[Philipp von Weitershausen [:philikon]]

Part 1: https://hg.mozilla.org/services/fx-sync/rev/3fd7feaf01db
Part 2: https://hg.mozilla.org/services/fx-sync/rev/355020081192
Part 3: https://hg.mozilla.org/services/fx-sync/rev/d7e68780d2e2
Part 4: https://hg.mozilla.org/services/fx-sync/rev/7080fdd0c506

Comment 12

[Philipp von Weitershausen [:philikon]]

Attachment: small follow up to fix copypasta

Comment 13

[Richard Newman [:rnewman]]

Comment on attachment 505193 [details] [diff] [review]
small follow up to fix copypasta

Looks like it should be there, so thumbs up.

(Confirmed in IRC that this fixes CW failures.)

(Still have no idea why it doesn't fail on my machine.)

Comment 14

[Philipp von Weitershausen [:philikon]]

Landed copypasta fix: https://hg.mozilla.org/services/fx-sync/rev/97b56ae1a5ce

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 504629 [details] [diff] [review]
Part 4: Combine auth.js and resource.js
[Checked in: See comment 15]

http://hg.mozilla.org/mozilla-central/rev/11935a5a21ad
without patch Makefile.in part.

Comment 16

[Serge Gautherie (:sgautherie)]

(In reply to Serge Gautherie (:sgautherie) from comment #15)
> without patch Makefile.in part.

s/Makefile.in/Makefile/

Comment 17

[Serge Gautherie (:sgautherie)]

Comment on attachment 504626 [details] [diff] [review]
Part 3: Combine base_record/* files into record.js
[Checked in: Seec omment 17]

http://hg.mozilla.org/mozilla-central/rev/50fd43f87ff4
without Makefile part

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 504624 [details] [diff] [review]
Part 2: Move store and tracker implementation into engines.js
[Checked in: See comment 18]

http://hg.mozilla.org/mozilla-central/rev/cb0da1fdfe9e
without Makefile part

Comment 19

[Serge Gautherie (:sgautherie)]

Comment on attachment 504619 [details] [diff] [review]
Part 1: Death to type_records
[Checked in: See comment 19]

http://hg.mozilla.org/mozilla-central/rev/636b5048b1a1
without Makefile part