Firefox/Projects/Tabcandy/Panorama 1 Debrief Chat

From MozillaWiki
Jump to: navigation, search

After Firefox 4 shipped, the Panorama team and other interested parties got together and discussed how things had gone. This is a transcript of that chat. You can also see the summary.

iangilman: So, welcome all to the official Panorama debrief!

zpao: helloooo

iangilman: I figure this can be a free flowing discussion

iangilman: howdy zpao

mitcho looks to iangilman to get us started with some structure, a guiding question, etc.

iangilman: anant, beltzner, dietrich, ehsan, juanb, msucan, not_gavin, shorlander, please join us if you have something to add!

iangilman: yup... I think I'll just dive in

msucan: hello everyone

iangilman: So, one of the obvious things about Panorama was that it was largely an outside team, not part of the mozilla establishment

iangilman: I think this was good and bad

iangilman: it was good because we had fresh eyes, and we were focused on just this one feature

iangilman: in fact, I think without a dedicated strike team, a large feature like this would never get done

iangilman: It was bad because we had a steep learning curve, and we made a number of naive decisions that had to be reversed later

mitcho: more institutional support would have been beneficial, though

iangilman: right, and maybe a single insider to help us navigate

mitcho: there were disconnects both in terms of code (a lot of us were new to mozilla firefox dev, so there was a lot we had to dig around to find ourselves) and also in project management/support, e.g. a main reviewer, platform contact, etc.

mitcho: yes

mitcho: at least

pcwalton: my biggest issue with the front end in general is review bandwidth

iangilman: on the other hand, we did have a number of good "friends of Panorama", like ehsan, mardak, and zpao, who really made it possible

mitcho: pcwalton: +1

mitcho: iangilman: true

iangilman: .... but we had to drag them away from their real jobs

zpao: i think part of the issue with the disjoint team was the lack of involvement at meetings (firefox team, platform). i think it's something that if it were in front of people more often, it would have gotten better attention

kevinh: interfacing with the Moz hive mind should have been done more frequently, and earlier

kevinh: not just for engineering, but across the board. In some sense, we played catch-up a lot.

pcwalton: I think all sub-projects should have a reviewer who can review things before they make it into at least a branch

pcwalton: patches sitting in bugzilla for months is the worst of all possible worlds IMHO

iangilman: zpao: good point! I think part of our problem is we didn't even know about those things

mitcho: we also weren't active in dev community involvement, like on planet.

pcwalton: get 'em checked in somewhere, even in a project branch

iangilman: it might be interesting to have a "so you want to start a feature strike team?" FAQ

mitcho: iangilman: good point

raymondlee: iangilman: yes, agree.

shorlander: I agree this is valuable insight for any future projects like this

zpao: definitely (for the team doing it and also for the people who should pay attention to the strike team)

mitcho: also, for me at least, it was unclear early on what exactly the hurdles would be in getting this in the product. whether this was really in-scope, whether we would have the support from the main product team, what the review and testing hurdles were, etc.

iangilman: pcwalton: getting me set up as module owner, so I could review patches, was a huge turning point

mitcho: early on, it was unclear how likely or unlikely it was that (a) we would ever land and (b) it wouldn't be pulled out

pcwalton: yes, that was really good

pcwalton: I wish the Web Console had had a module

iangilman: speaking of FAQ, I think our team wiki page worked out well: https://wiki.mozilla.org/Firefox/Projects/TabCandy/Work

kevinh: I see this conversation being echoed on the moz-dev planning group, but contributions from the community were really crucial for us

zpao: mitcho: that was an issue. partly because of lack of communication, but also because the schedule as a whole was in flux

kevinh: Making that process easier and more encouraged seems like both a good idea for strike teams and moz in general

mitcho: I feel that, if it was clear that this was a stated goal of fx4, a little earlier, within the firefox team, ux, and to dev/community communications, it would have been easier to get people on board and help

juanb: I don't know if there was a an assessment of the scope of the project in the beginning, which would have included people in the platform and front end team to sort out the doable vs future work within the timeframe of fx4.

iangilman: mitcho: indeed... it seemed like a couple of months of trying to land and not really knowing how far along the process we were... it was a spinning cursor, not a progress bar

mitcho: even having particular milestones which would be required for inclusion would have been helpful

juanb: I don't know that there was a set criteria of memory and performance metrics we could have used to check that we were doing ok, but that is also true of other projects.

mitcho: well, it seems that there are some criteria like that... don't hurt talos, don't explode memory, don't break things, etc... but that wasn't immediately clear at the beginning.

kevinh: seems like what I'm seeing is 1. we need better communication, preferably with a team member in MozHQ, 2. We need better road-mapping and planning, 3. We need better information on how to onboard people to the process. Most of that says PM to me, though I could be biased

ttaubert: hey everybody!, sorry I got stuck in traffic

iangilman: of course, if there had been too much thought put into whether Panorama should be part of Fx4, it never would have happened

mitcho: ttaubert: welcome!

kevinh: ttaubert: hey

raymondlee: ttaubert: hi

mitcho: iangilman: true.

iangilman: ttaubert: glad to have you here! I'm particularly interested in your perspective on how you found the project and getting up to speed this late in the game

ttaubert: oki

mitcho: iangilman: and to a certain extent it did take actually landing and then iterating to really convince more people that this was worthwhile, or so it felt

iangilman: Right... Panorama really only happened because we were so passionate we blew past all of the road blocks

msucan: i am working on some patches for devtools, so not very active in here, but my comments would be:

kevinh: and had a decent proof-of-concept add-on

raymondlee: Yeah, we didn't really know whether it would be in Fx 4 or not right at the beginning.

iangilman: right! Having a prototype people could see in action was huge for convincing people

raymondlee: the proof-of-concept add-on really helped.

iangilman: (perhaps aza will have more to say about that when he's back from his other meeting)

msucan: i think panorama's code is really nice. i dived into the code really late in the process, to help with a blocker, still it was easy to understand and dive into. i like the code, the structure, but it felt somehow too different from the code style of other mozilla js code

iangilman: msucan: excellent point... I actually find the mainstream mozilla code base really difficult to wrap my head around... h

msucan: panorama's code felt more like a typical web page

iangilman: ... so what's better? Having good clean code, or matching the rest of Mozilla?

msucan: with lots of js

iangilman: right... no xul

mitcho: on the other hand, though, I think there are issues with the way panorama is integrated with tabbrowser which happened *because* we built it first as an add-on and then a possibly-removeable addition to the browser

msucan: iangilman: i think clean code does not exclude matching the rest of Mozilla

iangilman: mitcho: good point... like what?

mitcho: i.e. the notion of "tab groups" should really have been a real xul/js construct in the tabbrowser level, ideally

mitcho: we still don't have that... only "hidden"

msucan: just follow code style guidelines. same structure, everything. just indent the same, use arguments with "a" prefix, and so on

mitcho: and that seems like a real pain point down the line, and is a clear reflection of this being developed as an "addition"

msucan: break lines like the rest of the code does

fryn: mitcho: as someone who works on tabbrowser code, i concur with you.

msucan: wrt. tab browser. it feels like panorama duplicates some stuff from tabbrowser

iangilman: msucan: I guess I don't really know the details on how our code is different. We did look at the coding guidelines and tried to follow them. This was, however, a while after we got started

mitcho: if, instead, the whole firefox team made it clear that panorama (or some kind of group functionality) was going to be a first class citizen, we would have built in groups in tabbrowser + sessionstore + sync early on, and then built the interface on top of that, like it should have been, imho

msucan: (but i didn't dive into "what could improve")

zpao: mitcho: i still feel the same way about sessionrestore. many issues came up because it wasn't tightly integrated

fryn: we may want to rewrite tabbrowser to be better modularized and be designed from the ground up to support the integration of features like panorama.

msucan: iangilman: for me it was confusing. you don't use lets, and so on. i had to change "my mode" for panorama

ttaubert: mitcho, right, it's a too defensive integration at the moment

kevinh: mitcho: I agree it wasn't ideal, but there IS a bootstrapping problem. I am not sure the Fx team would let in a project of such magnitude without first being proofed out

msucan: fryn: exactly

pcwalton: all I can say is that I'm glad that Panorama is a series of .js files and not a .xml like tabbrowser.xml

msucan: haha, me too

pcwalton: panorama code feels more like "modern" JS code

pcwalton: which

pcwalton: is unsurprising, given that it is... well, much newer

pcwalton: I like iQ. mad props to iQ.

kevinh: iQ is funny, some people really didn't want us to do it

mitcho: jeresig thought I was a little crazy

mitcho: when I told him about it

iangilman: Yeah?

iangilman: So obviously there are things Panorama still needs to take from mainstream mozilla

msucan: yep

iangilman: ... and sounds like there may be some things Panorama can contribute

fryn: because of panorama's uintentionally obfuscated hooks into tabbrowser code, i find that i often run into bugs due to how panorama overrides and changes the behavior of some getters in tabbrowser. i'll likely arrange a meeting or two to discuss that once i have more concrete ideas on how to address this problem.

fryn: *unintentionally

iangilman: like maybe there will be more html/js modules rather than xul? Maybe iQ could be used elsewhere?

mitcho: can we talk a little more about the integration of panorama before we get to good things about the process?

mitcho: (iangilman: though I agree with what you're starting to say...)

iangilman: I really like the idea of the core tech that makes Panorama possible being built into the guts of the browser (unlike how it is now) and the presentation of Panorama being just a thin layer of html/js that only reacts to and guides that core tech

iangilman: mitcho: sure... lead us

mitcho: so... (this sort of is counter to what I was just saying about better tabbrowser integration, but...) I think it was a mistake to not land it with a very easy code kill switch.

mitcho: especially as we didn't know whether it would really ship, even after landing

mitcho: there was a lot of community backlash at points, which may have been more easily alleviated if we had an about:config setting, at least.

kevinh: I actually think not caving to the killswitch was pretty key for us, as the reviews for us (on twitter, since launch) have been overwhelmingly positive

pcwalton: I think some of that was also due to a slightly overzealous land grab of keyboard/mouse shortcuts

pcwalton: most notably three-finger swipe

kevinh: the problem wasn't that Panorama was IN Fx, it was that it was broken, and too easy to trip on. I think we eventually struck a close-to-right, conservative balance on that

mitcho: sure

pcwalton: perhaps keyboard shortcuts should be phased in gradually as the product matures

mitcho: but for future projects, say

mitcho: what if we never improved the performance? for example

pcwalton: to avoid a backlash resulting from people accidentally triggering a feature that isn't ready yet

mitcho: it would have been a pain if months later we had to back the whole thing out

iangilman: looks like kill switches are going to be required for future features, for what it's worth (if you've been following moz-dev-planning)

ttaubert: sounds reasonable

raymondlee: should be easy to implement.

kevinh: The new structure might be better for new features. Iterate and polish on a separate branch until it is ready to hit m-c. Hopefully some perf work is part of that

mitcho: (not that we necessarily want to keep code "latent"... lord knows there's also other code which is in trunk but disabled... *cough*tabpreviews*cough*)

mitcho: iangilman: yeah

kevinh: pcwalton: very much so

zpao: (yea i think there's been talk of taking that out)

zpao: (though people us it so...)

mitcho: (yeah...)

mitcho: another thing on process:

iangilman: ttaubert: you're quiet today... what are your thoughts?

mitcho: starting with an add-on was very good for prototyping it for starters, but moving it from an add-on's "shell" into trunk, I think, also is a reason for why the final product is so "compartmentalized" in a sense

ttaubert: I'm reading and agreeing so far I slipped later into the whole process so that's not all new to me but some things are

mitcho: perhaps starting from a branch rather than from an add-on would have been better? though harder to get people to try it and get excited.

kevinh: maybe a tiered process, addon > branch > m-c

ttaubert: are branches contrary to kill switches?

mitcho: kevinh: that's essentially what we did, though

mitcho: ttaubert: perhaps.

raymondlee: mitcho: but would it be hard for people to try it if we were not an add-on

iangilman: mitcho: that gets into the question of whether we should have thrown out the prototype and written the integration from scratch

mitcho: ttaubert: but there's still a lot more you can do in a branch in terms of code organization that you can't with an addon

iangilman: .... thoughts on that?

mitcho: iangilman: I say yes.

mitcho: iangilman: probably starting from tabbrowser on up

kevinh: iangilman: it sounds like, for our needs, Fx's code should be easier to get TO from an add-on

ttaubert: I don't know the prototype but this makes sense

iangilman: I think I agree, although there's no way the existing team could have done that... we would have needed a dedicated "man on the inside"; probably multiple

mitcho: ttaubert: you should go find it. it's kind of fun.

mitcho: iangilman: true.

ttaubert: mitcho, ok

iangilman: ... in which case we would have been gated behind people's existing priorities, and it never would have happened

mitcho: ttaubert: don't think it'll work with fx4, though.

ttaubert: so at least in an ideal world we should have thrown away the prototype

mitcho: iangilman: unless, as discussed above, we actually had an employee or two dedicated to the project early on

mitcho: the prototype was indeed great for getting people excited and mocking this up, though

mitcho: kudos to aza!

aza: Man, this is a lot of scrollback.

iangilman: mitcho: I finally upgraded my primary browser to Fx4 now that it's released, and guess what? There was an ancient copy of the add-on sitting dormant in my profile... freaked me out!

ttaubert: (*sshhh* aza is reading

mitcho: hey seandunn !

seandunn: iangilman: good afternoon, sorry I'm late, long lunch.

seandunn: mitcho: hey!

mitcho: iangilman: hehe, maybe you can use that one too and get panorama-in-panorama, infinite loop style.

iangilman: seandunn: glad you could make it! We're still at it

raymondlee: hi seanunn

ttaubert: hey seandunn

raymondlee: seandunn ^

seandunn: raymondlee, ttaubert: hello!

iangilman: yeah, having a prototype is definitely a good idea, even if you do throw it away

ttaubert: yep

iangilman: the prototype allowed us to do all sorts of crazy experimentation at the beginning

ttaubert: I think that's a good way to try yourself out

mitcho: also in terms of integration and code flow... I think it would have been beneficial to start writing mochitests earlier, like when we started to move to a branch. looping in the mozmill/qa folks earlier would also have been nice, though a lesser priority, imho.

kevinh: ... which was easy enough to do that just iangilman and aza could riff on varied ideas at a rapid pace

mitcho: (although our code coverage is not bad!)

aza: Absolutely, an internal champion is necessary. Better integration with the Firefox team is right as well (my fault, really, for not being more on top of that). Trying to formalize the inclusion process would have been nice, but I don't actually think the Firefox team knew what those metrics are. Do, and ask forgiveness later, is the right way to go.

aza: I would have liked to get in front of the perf people earlier.

iangilman: right... if things get too formalized, then this sort of big feature may never make it

iangilman: it's a delicate balance

aza: Although the goals around less than 1% impact on Ts was really helpful.

iangilman: Now that aza's back, I'd like to call out another important factor: Aza's evangelism, especially the blog posts with videos

iangilman: I think getting such a huge public buzz on the outside helped us get action on the inside

mitcho: it was great, but I wish we'd done a little more on that.

iangilman: Also, I believe ttaubert found us because of this?

ttaubert: iangilman, yep I just wanted to say that

aza: With one caveat: my biggest learning this time around is never ever use public buzz as a way of trying to directly influence key decision makers within Mozilla. That bounces really poorly. Did that with Ubiquity to general badness. Let this buzzosphere speak for itself this time and it worked much much better.

mitcho: when I was working on ubiquity previously, I was encouraged to blog often about progress and thoughts, and that was pushed to planet, and a lot of people within the organization could latently follow progress that way

iangilman: Also the tweets with links to "good first bug" lists... seemed like a good idea

iangilman: ... not sure how much impact that had

mitcho: iangilman: I think it was good, though b.m.o is kind of scary, which is unfortunate.

aza: Although we let the "how to get involved" webpage languish, at the beginning it really helped with rallying. Similarly, not using Bugzilla to triage and plan at the beginning helped a lot too.

iangilman: aza: you mean never directly say "we have to do this, look at the buzz!"?

aza: iangilman, exactly

iangilman: mitcho: b.m.o?

aza: b.m.o = bugzilla.mozilla.org

iangilman: gotcha

iangilman: yeah, knowing at what point in the project to engage with the higher overhead of mozilla process

kevinh: probably when we graduated from 2-3 people to 4-5 people, actually

iangilman: oh, here's another difference in the Panorama codebase... we used naturalDocs-style comments, and actually used them to generate a help page: http://hg.mozilla.org/labs/tabcandy/raw-file/tip/content/doc/index.html

aza: Did other people find that useful in getting into our code?

iangilman: ... did anyone find those pages helpful? ttaubert? seandunn?

ttaubert: it's the first time I see it

ttaubert: I'm quite used to self-documenting code

pcwalton: I never saw it

pcwalton: but that's cool

ttaubert: +read

ttaubert: I appreciate it too, just didn't know it existed

seandunn: I never used it, although I probably would have ended up just looking at the code anyhow.

iangilman: fair enough

iangilman: I think it was useful for the original reviewers and the QA folks... aza?

mitcho: again, though, as nice as that was, it may have been beneficial in the long run to follow mozilla's commenting guidelines

aza: I wonder if the docs and adherence to having our code documented thusly held us to a higher standard.

iangilman: yes, sounds like it

aza: And yes, I think it was useful when brain dumping to our original reviewers.

aza: And moving forward, I'd assume it will help with modders

seandunn: iangilman: What I would have found much more helpful is a description on every class that, in detail, described all other classes it interacted with and how it was used. But, only when I was trying to grok everything.

iangilman: yeah, some sort of comment documentation in the code is good, but I think using the mozilla standard would be fine (though I think it would be great if the mozilla comment docs actually got turned into web page documentation)

aza: Another thing I found that the team did really well (especially coming from outside as we did) was form personal relationships with particular FX team members. That, I think, was absolutely necessary to the success of Panorama.

iangilman: hmm... I suppose we should have invited the read it later guy to this meeting... he was our add-on early adopter... would be interesting to have his perspective

kevinh: I've been engaged with him some of the last month or so

kevinh: he says our code was easy to dive into, and I believe, that the docs helped. He whipped up his Panorama integration in, what? A day?

iangilman: aza: how so?

iangilman: Like all the early help from mardak? and ehsan and zpao ?

iangilman: speaking of docs, we still don't have any official MDC docs for Panorama, nor do we have an API other than what naturally evolved

mitcho: creating the simple add-on sdk module for panorama integration was easy too (of course, that was me, though), though that's been on hold

iangilman: getting that landed will be great

mitcho: it would

iangilman: seandunn: would you like to talk about the review process? One of the things we've been discussing is the challenges of being sort of "outsiders" trying to make this happen

seandunn: sure

aza: iangilman: In terms of getting review. If we hadn't known dolske, ehsan, etc., then our crazy insane asks would have fallen on deaf ears.

iangilman: aza: for sure

raymondlee: Getting review was really a bottleneck of the whole landing process.

iangilman: speaking of which, #tabcandy has been crucial, and the fact that there are a number of browser core folks who are willing to just be here

seandunn: Any frustrations I had with the review process are in two areas: 1. The combination of lack of documentation on the "proper" way to do something combined with the iterative pain of satisfying a reviewer, and 2. the constant churn of code.

iangilman: Part of that bottleneck in the original landing was the fact that we had this gigantic wad of code... it would have been great to get the reviewers involved earlier.

mitcho: indeed

raymondlee: +1

iangilman: right... we had a number of occasions where we were review bound or approval bound, and a huge pile of patches would build up... when we finally got approval, everything was bitrotted

iangilman: I suspect these are issues for all of Firefox, not just Panorama, but definitely worth mentioning here

pcwalton: that's why I like project branches

iangilman: In someways we probably had it better than some other parts of firefox; at least we had advocates

pcwalton: having patches sit in bugzilla for a long time is a big drag on productivity

ttaubert: yep, I find it kind of problematic that waiting for review produced a big amount of context switches for everyone involved and reduced productivity everywhere

aza: Another complaint: One of the things I liked about early Tab Candy dev was that we were constantly fixing things and getting to a place of awesome dogfoodability. I.e., we were always polishing. As we got to the rush for features by feature freeze in FX that went by the wayside and the product suffered for it.

aza: ttaubert: *nods*

pcwalton: if they're landed *somewhere* then you have history, you have builds that people can try out, you have an ordering to prevent bitrot, you can take merges from trunk in an orderly fashion, etc.

pcwalton: for JS, anyone on the JS team can review patches before landing into tracemonkey

pcwalton: regardless of official module owner/peer status

pcwalton: it works great for them

iangilman: pcwalton: there's two ways you could take a project branch... review everything before landing there, so it's not a huge review process to land it on m-c, or review it afterwards; I can see advantages to both. for a big feature like Panorama (and perhaps even for the upcoming "all windows" work), it's possible you might even want two such branches, one for each. Or maybe that's just crazy

iangilman: pcwalton: sounds cool

iangilman: aza: we kind of ended up in "ship emergency" mode in september and stayed that way for months...

kevinh: our triaging was, like all of Fx, pretty brutal

iangilman: part of our challenge there was incompatibility with new things that had just landed in Firefox, like app tabs and sync

mitcho: iangilman pcwalton: I agree, the tracemonkey approach is interesting

ttaubert: yep

iangilman: ashughes, Mil, philikon: anything to add, or just observing?

iangilman: seems like we're winding down here

kevinh: One thing we could have done earlier was start to plan for transition. We're at a stage now where it's hard to say who will be working on Panorama.

ashughes: iangilman: I wasn't paying attention, to be honest

iangilman: good point!

mitcho: kevinh: +1

ttaubert: +1

iangilman: ashughes: no worries... just curious if you have any thoughts about what went well or poorly in the process, from where you sit

philikon: iangilman: sorry, was just observing

kevinh: and in general, should walled-off projects like Panorama get integrated into the greater moz-dev community?

juanb: From a QA perspective and probably others, it would have been the awesome sauce if we had clear use cases. While watching the tabcandy component in bugzilla I often felt like we were defining behavior along the way, which is fine because things change, but I'd get a little lost.

philikon: but i agree with project branches!

kevinh: ... or should we keep a small dedicated team?

juanb: "how should tab candy work in private browsing?" and then I'd just try to use common sense.

iangilman: juanb: I think we didn't know either! Probably having a common place we could continue to evolve the "spec" would have helped

ashughes: to carry off of juanb thoughts, I'd like to add that while that is true, I found developer response on bugs to QA requests was both timely and efficient

mitcho: iangilman: +1 on spec

juanb: Dev response was great.

iangilman: philikon: no worries. I'll take that as a sign that we didn't aggravate you too much during the last year

juanb: Without you having that much of mozilla experience, you were very interactive here and in bugzilla.

iangilman: ashughes, juanb good to hear!

iangilman: So having well-defined points of team accessibility is good

iangilman: How about our weekly "scrum"? Did the team find that helpful? It was pretty casual...

raymondlee: yes the weekly scrum is a good thing to know what others are doing.

mitcho: twas good

iangilman: Cool

raymondlee: especially, I am in different time zone

iangilman: raymondlee: any other thoughts on how we did with the timezone thing?

ttaubert: I like(d) it, too

iangilman: ttaubert just solved it by staying up all night

ttaubert: haha

ttaubert: sad but true!

iangilman: Not really a viable, long-term, plan, though

ttaubert: but actually working in a different timezone is not that bad

ttaubert: if you've got a todo list you can work on that over the day

mitcho: the timezone, and the remote-ness, forced us all to make sure to communicate well via scrum and bugs

raymondlee: yes, a todo list is a good thing.

ttaubert: the last 2-3 hours of the day can be spent with team communication and getting some reviews

mitcho: of course it would have been great to be all in the same room :), but there's an upside

ttaubert: so you know what to start with the next day

iangilman: yeah, I really liked how the project was moving forward 24 hours... I'd always wake up to a pile of new stuff

iangilman: So, mitcho has suggested we also use this time to look forward to next steps as well... sound good to folks? Any other backward looking items anyone wants to cover first?

raymondlee: One thing I like it's the bugzilla emails so I could read all those emails in the morning and knowing what happened while I was sleeping.

mitcho: raymondlee: same here.

ttaubert: yep that's great

mitcho: often from bed

ttaubert: indeed

iangilman: Yeah, I have to say bugzilla is horrifically ugly and clunky, but I've grown to love it

iangilman: ... and the email is a great way to know what's going on

kevinh: which I think speaks to how useful it is to have smaller, modularized, features

mitcho: atul's bugzilla dashboard didn't hurt

kevinh: we didn't catch the firehose from the rest of bugzilla, we only got Panorama stuff

iangilman: alright... let's call the debrief to a close... thank you for all the great thoughts! Kevin and I will distill this chat down and add it to the wiki, probably also a planet mozilla blog post