=== Ursinha-bbl is now known as Ursinha [00:59] hello [01:00] #startmeeting [01:00] Meeting started at 19:00. The chair is bac. [01:00] Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [01:00] hi guys [01:00] * rockstar [01:01] * StevenK waves [01:02] * wallyworld__ waves back [01:03] [topic] outstanding action [01:03] New Topic: outstanding action [01:03] [topic] Sinzui to investigate making lint check for the Storm 'in' gotcha [01:03] New Topic: Sinzui to investigate making lint check for the Storm 'in' gotcha [01:04] last week we talked about using the Python 'in' operator with Storm and how it is Bad. sinzui reports pretty good progress on changing lint to sniff these occurrences out. yay. [01:04] [topic] HenningE to add a note to the PSG about the use of any [01:04] New Topic: HenningE to add a note to the PSG about the use of any [01:04] 'any' and 'all [01:04] actually. henning wasn't at the meeting so we don't know if he got around to it [01:05] [topic] Mentat update. [01:05] New Topic: Mentat update. [01:05] stevenk, how is the mentoring process going? [01:05] Quite slowly, sadly. thumper was on holidays and I forgot I was OCR, so that didn't help/ [01:06] StevenK: what is your day? [01:06] Wednesday AsiaPac [01:07] ok. i'll try to throw my next branch at you. [01:07] Sounds good. :-) [01:07] StevenK: when you have spare time you can always pull branches off +activereviews [01:08] It's the spare time of that sentence that's getting me too. [01:08] hi lifeless [01:09] recall too that henning and salgado are UI review mentats. please consider them first for your UI reviews. [01:09] and also note we lose noodles as a UI reviewer as he moves to ISD [01:10] does that mean we should explicitly specify those guys when requesting a review instead of leaving it up for grabs? [01:10] wallyworld__: yes, that would be good if you do [01:10] did I miss some context? [01:10] lifeless: mentored reviewer progress [01:11] so bottlenecking on two folk makes me wary [01:11] could we just say that 'all nonUI reviewers are UI review mentees' [01:12] and then if you happen to have a mentee on a branch with UI stuff that the mentee doesn't feel capable on, they can call in a mentor? [01:12] well, if he were a mentee then by definition he'd have to call in a mentor to approve the branch [01:13] rockstar what have we said about moving everyone towards being UI reviewers? [01:13] is bottlenecking on ui reviews a problem people are having? [01:13] as someone who touches the lp ui about once a year, i have no idea :-) [01:13] bac, well, after chatting with sinzui, I think henninge can probably graduate. [01:14] bac, I think I was pushing for everyone being a javascript reviewer first. [01:14] rockstar and i think salgado must be close [01:14] I will admit that I've just ignored needing/not needing a UI review; but I suspect I'm a special case in that regard [01:14] bac, unfortunately, after my recent javascript work, I kinda want to go the other way now. :) [01:14] lifeless, how are you a special case? [01:15] rockstar: either I've never made a UI change that needed a UI reviewer (and I don't know what triggers that clause) [01:16] rockstar: or noone has felt comfortable telling me they think a branch will need one [01:16] lifeless, usually, your reviewer should tell you. [01:16] rockstar: or I've had a UI reviewer by chance when I've needed one [01:17] bac, ideally, everyone is a everything reviewer. [01:17] I think we should just work on that basis [01:17] lifeless, unfortunately, I can't agree. [01:17] and if someone's review isn't catching what we care about, we act on that and help them improve [01:17] rockstar: why not? [01:18] lifeless, I think (currently) most people's reviews aren't catching what we care about. [01:18] rockstar: I agree. For /all/ review types. [01:18] lifeless, at the Epic two years ago, we made a big push to start doing more javascript. Unfortunately, most people are avoiding that. [01:19] FWIW, I think reviews are at the moment fairly useless. [01:19] They mostly seem to check stylistic issues, but rarely anything functional. [01:19] last week we talked about what reviews should be doing; I'm working on a clear articulation of the issues I see there. [01:20] wgrant, yeah, that's what lifeless' arch guide is about. [01:20] i find most of my review comments are insisting that comments are accurate and make sense [01:20] i think this is still valuable though [01:20] * rockstar has that in mind when he does reviews. [01:20] rockstar: cool [01:20] rockstar: so, I still don't understand your objection [01:21] we suck at js? [01:21] mwhudson wins, methinks. [01:21] if most reviews are not getting at things we care about, it seems like to be one of a few things: [01:21] wgrant: do you feel the quality of reviews has gone down lately or have they always been superficial and mostly useless? [01:21] - insufficient training / depth of knowledge / breadth of knowledge [01:22] - fear of getting deep in [01:22] - insufficient time to do the review well [01:22] bac: It hasn't changed lately. [01:22] - ??? [01:22] - profit! [01:22] But, well, I haven't been around long. [01:23] bac: they weren't always superficial, but there were problems centred around having very large pieces of work arrive all at once with very poor internal structure [01:24] lifeless: harking back to last week, i think an issue is that for the things we care about now, review is too late [01:24] the desired solution to that - smaller bits of work - has seemingly resulted in shallower reviews [01:24] lifeless: i was about to raise that issue [01:24] mwhudson: I don't think its too late per se. [01:25] lifeless: your way of putting it makes a lot of sense too [01:25] smaller units of work mean it is harder to cohesively see the bigger effort [01:25] mwhudson: but it sure is hard if the incoming branch has *lots* of room to improve :) [01:25] 8 small reviews do not, in some sense, make up a good review of the one large feature they all implement [01:25] right [01:25] well hurrah, we all agree on this point :-) [01:25] and I'm not about to suggest that we go back to the good^Wbad ol' days [01:26] it would be an interesting experiment: i wonder how much effort on general code quality the simple knowledge that you have to get your code through review has [01:26] would a solution be to have a larger unit of work split into smaller chunks, but insist on the one reviewer all the way through to keep the bigger picture aspects in mind? [01:26] effort? impact? [01:26] of course the problem is made worse when the n reviews for the n smaller branches are spread over n reviewers [01:26] mmm [01:27] maybe i should try to find a CS lecturer to do this experiment... [01:27] so LP is small enough for one developer to whole a reasonable approximation of the entire system in their head. [01:27] and we have dedicated periods for review when folk don't have to be simultaneously cutting new code. [01:27] I think that's a fallacy [01:28] I have trouble keeping all of the bits I care about in my head, so I don't see how anyone could keep track of the whole of Launchpad, even to a reasonable approximation -- we also move far too quick for that [01:28] it may be false to say that all lp developers can do that. [01:28] StevenK: we move glacially. [01:29] As a whole, yes [01:29] Certainly aspects are changing quicker [01:29] i think lp is a bit too big to hope for many people to understand how it all fits together [01:29] s/ly// [01:29] I'm going to take a second to put some thoughts together [01:33] ok [01:33] so here's the thing. [01:33] either the system is so big that we can no longer tell whats in it. [01:33] In which case I think we need a rule like drizzle had(has?) - no patch can make the codebase bigger. Smaller only. Fin. [01:34] Or the system isn't that big. [01:34] hmm [01:34] In which case I think its reasonable to ask that reviewers be getting the big picture, looking for things that should have been refactored, things that will scale poorly, etc. [01:34] partially baked thought, but [01:35] it's not really size that's the thing to worry about, but complexity, and in particular, close coupling [01:35] mwhudson: inappropriate coupling for sure.... sometimes close is a benefit. Sometimes not ;) [01:36] mwhudson: cohesion + close-coupling are two metrics I would grab offhand for talking about complexity. [01:36] "inappropriate coupling" is new on the BBC, i think [01:36] lifeless: i guess -- i mean stuff like when code developer A reviews code developer B's code and then breaks translations [01:37] mwhudson: yes, that sucks; [01:37] * lifeless puts some more coal the mwhudson thinking bbq [01:37] s/the/ on the/ [01:38] :) [01:38] as per usual in an unclear discussion, perhaps we need to ask "what is the problem here" [01:39] most reviews are perfunctory: not /really/ a problem [01:39] mwhudson: its a problem [01:39] reviews are wasting developer time and reducing velocity: that's a problem [01:39] that too [01:39] (sorry, didn't mean to break your flow; fingers-up) [01:40] lifeless: or another way, perfunctory-ness is a problem, but it requires looking a layer in [01:40] i recall back in 2007 when it would sometimes take days to get your code reviewed; it's a lot better than that now [01:41] (unless you're jam :/) [01:41] mwhudson: now it takes days to land it :) [01:41] yeah [01:41] that's certainly a problem [01:41] * mwhudson gets off the soapbox [01:41] so I think reviews that miss crucial things like 'will blow out the query count for the home page' are a problem. [01:42] mwhudson: the slowness to get reviews back then certainlly wasn't due to more thoroughness of the review [01:42] bac: no, it was due to idiotic processes :) [01:42] and reviews that happily add a duplicate (nearly line for line) result set decorator likewise. [01:42] right [01:42] but we were talking UI and javascript [01:43] we have formalise the idea of: [01:43] - code reviews [01:43] - db reviews [01:43] lifeless: for the latter, good code organization probably help [01:43] - ui reviews [01:43] s [01:43] - javascript reviews [01:43] we have an idea of structural/architectural reviews which I've introduced as an element of code reviews. [01:44] as you say, a review that slows velocity and doesn't add compensating value is a problem. [01:44] are there other problems that folk would like to note, about reviews [01:45] I'll take that as a no [01:45] so for every *different* review type we require a change to go through we create a handoff [01:45] we've recently shifted the 'qa review' (typically self-inflicted) from being arbitrarily close to release to being a pipeline stall for everyone, for instance. [01:46] we've justified that by saying that the risk of a bad deployment outweighs the cost of having a stall [01:47] on all our review types, we don't prevent all issues entering the codebase (note that I'm not saying we -should-) [01:47] but if its a sliding scale (I think it is), why can't we get more folk involved in the reviews, training up the scale now ? [01:48] what do you mean by getting more people involved? [01:48] bac: Why can't stevenk, for instance, do a javascript review, and: [01:49] - if he feels out of depth, punt (but watch the review to learn) [01:49] lifeless: that is exactly the process for JS [01:49] is it the same process for UI? I was told here by you that it wasn't... [01:49] it is not [01:50] why isn't it? [01:50] b/c historically UI changes were jealously guarded by our UI designer. [01:51] do we have a UI designer now ? [01:51] the review point was abused as the time for actually doing UI design [01:51] lifeless: no, we do not [01:51] then I think we should change this. [01:52] if we were to make it the same as JS; what risks would we face? What benefits would we get? we probably need to think about the risks reasonably closely. [01:52] we do have a small set of UI reviewers who are supposed to enforce not just a nicely designed UI for a given page but one that fits into the larger system [01:52] are UI changes qualitatively meeting that goal? [01:53] at a higher success rate per change than other sorts of changes in the system? [01:53] unmeasured [01:54] fwiw the feedback I hear is that the LP ui is confusing and strange [01:54] should ui changes/additions go through an approval process by ui gatekeepers at the *design* stage? using balsamic etc? [01:54] it's probably better than it was though [01:54] my personal and professional opinion is that UI is driven by the entire stack. [01:54] +1 on the lp ui comment from lifeless [01:54] trying to make it nice at the top end is a world of pain. [01:54] wallyworld__: i think that would be much better [01:55] guys i hate to duck out on a great conversation, but we are over time and i have another commitment [01:55] unless the defintion of UI is -extremely shallow- [01:55] wallyworld__: I may misunderstand but isn't balsamiq just a mockup tool? [01:56] feel free to stay and chat and i'll read the backlog [01:56] yes, but it can be used as a tool to help communicate ui design etc for approval [01:56] so this comes back to mwhudsons point about *when* we decide various things [01:57] do we decide to have a composite or inheritance in review or 'preimpl' or 'as we go' [01:57] wallyworld__, we set out, as a team, a process for using balsamiq for mockups - it's the easiest, fastest way to try and iterate upon and idea [01:57] yes, review time is way too late for lots of things [01:58] replace those terms with whatever ones make sense for e.g. db, javascript, language, etc etc [01:58] wallyworld__, and we really like to do mockups before writing code - Michael Nelson and his work on the packaging UI is an exemplar of the technique [01:58] wallyworld__: by way to late, do you mean 'it can feel very hard to change those things after doing some work on them' [01:58] i think we are saying the same thing with balsamic [01:59] the main point with mentioning balsamic is that i was trying to say we need to use various tools to communicate ui changes for approval *before* code is written [01:59] I'd like a better definition of 'too late' [01:59] "too late" = the cost of fixing issues becomes too great [01:59] wallyworld__: I think thats a pessimisation and we can do better; it adds handoffs and roundtrips that are poorly justified. [02:00] * wallyworld__ thinks [02:00] it's all a balancing act. handoffs are bad but so is trying to fix stuff too late in the process [02:01] wallyworld__: I'm thinking -very- big picture here: approval meaning 'passes whatever criteria we as a team have decided on', and the topic being any of ui/js/python code/db/component choices/... [02:01] What I'd like to see is, broadly: [02:01] - stop treating all changes as homogeneous (they aren't) [02:02] - the amount of approval needed should vary with the impact of a mistake [02:02] i'd the the *timing* of approval should relate to the best place to identify and fix mistakes [02:03] mmm [02:03] i meant "i'd add..." [02:03] that leads to the BDUF set of issues [02:03] I'm entirely happy with the goal of having such discussions as needed to be move forward early [02:04] but approving before its done is a great way to fail to adapt to changing needs [02:04] doesn't have to mean BDUF.... [02:04] lifeless, "BDUF"? [02:04] big design up front [02:04] ah, of course [02:05] we just need to be pragmatic and use our skills as experiences software engineers to do "what's right" [02:05] wallyworld__: lets set a goal: 4 hours to concieve and deliver *into production* a change. [02:05] wallyworld__: if you can fit preapprovals into that, I will happily tolerate them. [02:05] surely that 4hr window depends on the scope of the change? [02:06] and at the moment, that whole 4hrs would be consumed by the landing process :-( [02:06] wallyworld__: if you don't break things up into small pieces yes. And that comes back to the scaling points I made above. [02:06] wallyworld__: yes, but don't let currently problems drag you down :) [02:07] ah, but to have the small pieces to implement, you often need to have the bigger picture all sorted so that it can be chunked up [02:07] and you often need approval/review of the bigger picture design [02:07] before it is to be split up into units of work [02:08] wallyworld__: well, wearing my TA hat, I can say that I won't approve things at that level: I will discuss and collaborate, and reject things that are clearly problematic. [02:09] well that's review rather than approval. so long as those biiger picture things are collaborative and have more than one set of eyes on them [02:09] wallyworld__: but I'm not interested in saying 'this is definitely ok' for 2 months work - I think its a poor idea (and lots of literature on estimating work, and the overal XP/agile community agree AFAICT) [02:09] +1. see my previous comment [02:09] you just want to stop any obvious failings before they have a chance to go too far [02:10] right [02:10] and perhaps ask some pertinant "have you thought about this or that questions" [02:10] I've said it before, I'd be delighted to see much more design traffic on the list. [02:10] to minimise the chances of issues down the track [02:10] designs seem to be happening largely in very private discussions and then we hit the 'its hard to change this now' complaint at review time [02:11] yeah, it can be hard to get a response to this [02:11] i agree that's bad and we should encourage a culture where that doesn't happen [02:11] mwhudson: 'this'? [02:11] perhaps people are scared to make a mistake or look bad in front of the entire list? [02:11] lifeless: sorry, bad phrasing [02:12] so it's easier for them to do design stuff in a smaller group? [02:12] mwhudson: de nada, just couldn't bind it sensibly. [02:12] lifeless: basically, design proposals to launchpad-dev are often greeted with a deafening silence [02:12] wallyworld__: look at what I do for instance; small stuff I JFDI, larger stuff I do a spike on / mail the list for discussion. [02:12] maybe we need an "on call design opinion" as well as on call review [02:13] mwhudson: +1 [02:13] mwhudson: studies on group intelligence suggest that the dominating ability to be 'smart' as a group is getting everyones opinion. [02:13] so -1 on on-call [02:13] for example, i was the only person to respond in depth to one of lifeless [02:13] ' design mails, and i'm not even on the team any more [02:14] lifeless: put it another way [02:14] I think it would be great to use the OCR as a teddy bear, if thats what you meant [02:14] back in the lean training, i remember advocating the idea that doing reviews was at least as important as doing your 'own' work [02:14] argh, one more step back [02:15] it would be interesting to have everyone read every commit. [02:15] hypothesis: people don't reply to design mail because they don't feel they have the time/energy to really dig into it and respond usefully [02:15] for a week. [02:15] mwhudson: i'd like to provide more input but feel i need to learn the lp architecture a bit more first :-) [02:15] wallyworld__: please do, don't hold back. [02:16] be careful what you wish for :-D [02:16] proposed solution: promote a culture where replying to design mail is considered a really good way of spending your time [02:17] mwhudson, did we not have the technical architecture group and [tech] (I think) mailing list tag for that? [02:17] hence my analogy to reviews -- similar problems were held up wrt getting reviews done back in 2007 [02:17] +1. atm i personally find it really hard to keep up with the volume of email and get my "core" work done [02:17] i.e. not enough time [02:17] mars: the t-a-g is a design failure - it adds filtering to what should be a core dev responsibility [02:17] ocr + cultural changes helped there [02:18] on call design (ocd? :-p) may not be the thing here [02:18] I think the dominating driving factor though is test suite time. [02:18] mwhudson: Not enough time, or infrastructure issues (slow tests, third-time's-the-charm landing) making it ridiculously inefficient? [02:18] but cultural change possibly [02:18] it more than anything else saps time and adds to the pressure to be *doing* not *talking* [02:18] wgrant: hey, a 4 hour test suite gives you _masses_ of time to reply to design mails :-p [02:19] True, true. [02:19] mwhudson: I agree that cultural change is needed too; see for instance flacostes thread recently. [02:19] lifeless, I think one goal was to have a group of interested and capable experts from the LP team who could reply to design decisions. The [tag] changed the social contract to be "must reply" for that group. [02:19] mwhudson: not when something fails and you burn time looking into why and find out it's not your stuff after all :-( [02:19] lifeless: yes, making dev generally easier and smoother will help [02:19] wallyworld__: yeah, i wasn't being very serious there [02:19] mars: I want input from the least experienced of our engineers as much as the most. [02:19] i know, just being mischievious [02:20] s/m.*$/aussie/ [02:20] :) [02:20] * wgrant stomps on lifeless. [02:20] i need to duck out now too [02:20] me too; lunch calls [02:20] I'd like to highlight one really important bit that this discussion clarified for me. [02:20] lifeless, well, the group was to prevent the deafening silences mwhudson spoke of. Anyone was free to opine in the thread [02:21] 'its too hard at review time' really means 'engineers find changing things at review time hard' [02:21] Personally, I don't find it hard. [02:21] It just sucks to rewrite something you just wrote. [02:21] pre-landing review is *entirely* capable of identifying ui glitches, js bugs, code structure issues, performance issues. [02:21] i'd rephrase as "it's too hard to fix a bad design issue onces it's already been coded" [02:21] it shouldn't happen at review time. [02:22] wallyworld__: the bit I object too is 'too hard' [02:22] And I should have brought this up at the meeting, but consistency! [02:22] It probably just needs to be easier to grab someone mid-implementation. [02:22] wgrant: Using *gasp* WIP MPs [02:22] wallyworld__: its *not* too hard. [02:22] * StevenK glares at lifeless [02:22] wallyworld__: its underdesirable that the *first time* an issue is encountered is *after a lot of work is done* [02:22] StevenK: Well, bouncing to Julian introduces far more latency than would generally be ideal. [02:22] lifeless: s/too hard/bad practice [02:23] wallyworld__: but we have *two* concerns [02:23] a) engineer efficiency [02:23] b) gatekeeping [02:23] actually [02:24] agreed. and i think we could consider more gatekeeping earlier in the development cycle [02:24] b) maintaining 'quality' of various aspects (code/perf/ui/schema/..) [02:24] lifeless, bingo [02:24] we can do many things to b: [02:24] lifeless, you forgot scheduling and time pressure [02:24] - split it up [02:24] sometimes people don't have time to change how they did things (I've gotten hard pushback on reviews for that before :( [02:24] - move it after landing in trunk [02:24] - make it optional [02:24] for sure making development easier (faster more reliable pre-merge tests, daily rollouts) and reducing the cost of mistakes (feature flags!) will make everything else way better [02:24] - move all or some of it earlier in the dev cycle [02:25] * mwhudson really disappears [02:25] but in terms of deciding what to do; I want us to have a *clear* vision of 4 hour dev cycles: two dev cycles a day. [02:25] Thats the LEAN inner loop. [02:25] we shouldn't do tings to b) that will interfere with reaching a 4 hour cycle time. [02:26] lifeless: for you that's 6 hours answering everyone's questions on irc and 2x 1hr dev cycles [02:26] wallyworld__: its the price I pay for this amazing job [02:26] But he's lifeless. [02:26] Haha [02:26] So that might even out. [02:26] wallyworld__, he's from the SCRUM camp :) [02:27] oh that explains a lot :-) [02:27] mars: not really ;) [02:27] mars: some of the scrum stuff is just nuts [02:28] ok, fooding, thanks for the awesome discussion. [02:28] lifeless, I meant in that you spend your time removing obstacles for other engineers so they can continue their work - the scrum master role [02:28] later === Ursinha is now known as Ursinha-afk === Ursinha-afk is now known as Ursinha === salgado is now known as salgado-dr === salgado-dr is now known as salgado === _thumper_ is now known as thumper