Right now calendar's dependency management situation leaves room for improvement. In order to (1) make sure we're loading things lazily where possible (2) express the dependencies between our different modules clearly we should use something like es6 modules, requirejs, or browserify. Requirejs has quite a bit of momentum amongst other apps like camera, clock, and email in gaia and since several gaia people have also contributed deeply to the rjs community/ecosystem, I suggest we go for that.

Looking for owner / peer agreement (or disagreement)

Support using RequireJS in Calendar. We could use shim config[1] to let non-AMD and AMD code live in Calendar at same time. Then we could refactor the code partially to achieve the objective of fully using RequireJS in Calendar. [1] http://requirejs.org/docs/api.html#config-shim

No objections to me as long as we don't regress performance (we can probably improve it more with rjs)

As for the performance parts, it may be useful to implement the startup events first to get a baseline, I believe bug 837671 for calendar. The one we did for email was bug 837677. It feels like those events will better record the important markers in the app startup than what is used now. The existing datazilla cold start for email for example does not accurately measure use of the cookie cache, where with the startup events we can send the event at the right time.

now that I know enough about the startup process and added the new performance events I also want to do this and replace the old preloader logic! AMD FTW!

Go Miller!

still work in progress (and needs to rebase it since codebase changed a lot the past month), but adding it here so we keep a track of progress and avoid duplicate efforts.

https://github.com/mozilla-b2g/gaia/commit/3e63fe47475c9c00dfb0f98fbb3fc22b9fefd500 landed on master

Sorry to be the bad guy here, but unfortunately due to bug 1075787 real test failures were hidden from us, and this causes a perma marionette failure. It looks like this is causing some integration errors such as: https://tbpl.mozilla.org/php/getParsedLog.php?id=49341402&tree=Gaia-Try#error1 The likely reason is that these integration tests are leveraging the Calendar app for notifications. Once we get the tree green I will be more than happy to work with you to get this re-landed. Backed out: https://github.com/mozilla-b2g/gaia/commit/cbeddf48999e3c713570bbac8138f21515316103

So we have facilities in marionette-js-runner to create mock apps for cases like these which is my suggestion here? Shall we disable those tests, reland the amd patch, and then get a bug to rewrite those tests without coupling them with the calendar app implementation?

(In reply to Gareth Aye [:gaye] from comment #10) > So we have facilities in marionette-js-runner to create mock apps for cases > like these which is my suggestion here? Shall we disable those tests, reland > the amd patch, and then get a bug to rewrite those tests without coupling > them with the calendar app implementation? I'm fine with either solution in the end. I don't mind if two apps use each other as long as they use a shared JS library as this is generally much less code to write, and I'm probably guilty of doing this :) If we are going to use a mock app, let's please do so before disabling the tests.

https://github.com/mozilla-b2g/gaia/commit/56d949e8d1c66abb5ee2a0188eac6c0974417212 landed again after fixing system app integration test

Can we get a back out of https://github.com/mozilla-b2g/gaia/commit/56d949e8d1c66abb5ee2a0188eac6c0974417212 ? I believe it's causing Calendar app to fail launching. See Bug 1078244.

(In reply to Viorela Ioia [:viorela] from comment #13) > Can we get a back out of > https://github.com/mozilla-b2g/gaia/commit/ > 56d949e8d1c66abb5ee2a0188eac6c0974417212 ? > I believe it's causing Calendar app to fail launching. See Bug 1078244. reverted in https://github.com/mozilla-b2g/gaia/commit/c3800a31b86648e10fc9ee4dac9673856df1a1f2 - hope thats helps.

I'm not sure that bug 1078244 is valid. It's true that the calendar views were taking longer to load after the patch that you all backed out. It wasn't outright failing to launch though. This is probably an automation issue (and perhaps a calendar startup performance issue); I don't think calendar was completely bricked after that patch landed. All of the js marionette tests were passing (I think we have over 50 test cases at this point) and things were working as expected on my flame at master. I'm going to check to make sure that I can indeed make an event in each calendar view with the patch and, if I can, I'll go ahead and reland, disable the python test, and file a followup to fix the test.

Did we test to make sure a PRODUCTION=1 and MOZILLA_OFFICIAL=1 build works?

Gaye, this is not a test issue. The calendar app fails to launch manually too and after the backout it works fine. We tested a revert before requesting the backout. It's a device only issue too because all of Gij and Gip are green.

Interesting... this could definitely be a build issue since we are now using the rjs optimizer and I haven't tested with PRODUCTION=1 and MOZILLA_OFFICIAL=1. Will look into that and thanks for the help Kevin and Zac.

I tried to launch Calendar app on a build that includes the back out and I still see the issue described in Bug 1078244. So this patch did not break the Calendar app. Sorry for asking a back out, that was a mistake. Can you re-land this or maybe help us find the cause of this issue? Thanks!

Ruh roh! You guys didn't back out the whole thing. When you revert stuff, you have to revert merge commits in case there are multiple child commits. In this case there were two child commits :/. I will try to clean things up...

So this pull request reverts the bad revert and adds a clean revert on top... https://github.com/mozilla-b2g/gaia/pull/24843

Tested and calendar launches appropriately even with PRODUCTION=1 set after clean revert. https://github.com/mozilla-b2g/gaia/commit/83de447d9ae9a59459d7a445f9348a254c661850

The issue is fixed, Calendar app works as expected. Thanks a lot for your help, Gareth!

[priority] --> tracking-b2g:+ conversion

landed into master a few days ago, and this time we did not back it out \o/ https://github.com/mozilla-b2g/gaia/commit/0565fd0735bd353fda9b355c197d9f656907b1b4

Today a few testers were testing the calendar app on Flame 2.2 specifically and listed multiple tests that they tried as well as results for each test: http://bit.ly/1FRcz5C A couple of bugs were found, and can be found here: http://mzl.la/1zeQLze A few minor bugs were found as well as 1 performance bug. Overall the calendar app seems to be functioning well and the calendar has almost no noticeable performance problems. Flame 2.2 Device: Flame Master (319mb)(Kitkat Base)(Full Flash) Build ID: 20141028040202 Gaia: 6a7fb482a03c5083ef79b41e7b0dfab27527cd04 Gecko: a255a234946e Version: 36.0a1 (Master) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0