=== mrevell is now known as mrevell-lunch === salgado-afk is now known as salgado === mrevell-lunch is now known as mrevell [13:59] * bigjools is waiting with anticipation [14:00] #startmeeting [14:00] Meeting started at 15:00. The chair is barry. [14:00] Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [14:00] welcome everyone to this week's ameu reviewer's meeting [14:00] who's here today? [14:00] me [14:00] me [14:00] me [14:00] me [14:00] me [14:00] me [14:00] me [14:01] me [14:01] gmb sends his apologies [14:01] [TOPIC] Agenda [14:01] New Topic: Agenda [14:01] == Agenda == [14:01] * Roll call [14:01] * Next meeting [14:01] * Action items [14:01] * Queue status [14:01] * Mentoring update [14:01] * Review process [14:01] * '''Pre-imp calls are falling by the wayside''' (gmb) [14:02] schwuk has a family emergency as you guess you saw [14:02] me (sorry for delay) [14:02] bigjools: i did :( [14:02] [TOPIC] next meeting [14:02] New Topic: next meeting [14:02] same time and place week += 1 ? [14:03] anybody know they won't be here? [14:03] done [14:03] [TOPIC] action items [14:03] New Topic: action items [14:03] * gmb to hack review-submit to enforce 800 line limit. [14:04] don't think this was done, so we'll just push it to next week [14:04] * schwuk to work with mwhudson to get instructions for running loggerhead onto the wiki [14:04] anybody know anything about this one? [14:05] if not, we can just push it to next week too [14:05] barry: I do [14:05] sinzui: the floor is yours [14:06] barry: sorry, I thought the remark was about why it has not landed [14:06] um, which the 800 line limit one? [14:07] mwhudson's loggerhead branch that precipitated the need for instruction about how to run it. [14:07] oh [14:07] sinzui: is this so we can run loggerhead on our dev boxes? [14:08] barry: yes. I have not seen the instructions yet. [14:08] sinzui: but there was something about this that didn't land? [14:08] barry: never mind me, I'll sit down and shutup [14:08] ok :) [14:08] i guess we'll move on :) [14:08] [TOPIC] queue status [14:08] New Topic: queue status [14:09] i see lots of 1.2.4 branches in the queue and it being week 4 i guess it's good there are no 1.2.3 branches there :) [14:10] we do have 5 pink branches in needs-review, anybody know anything about those? [14:10] (there are 3 pinks that have been merged; i guess PR hasn't been updated for those yet) [14:10] 3 of the unmerged pinks have conflicts [14:11] PR gets updated a bit less now that it's needed less, I think :) [14:11] 2 are stub's branches, i'm guessing that's just the long running work stuart's been doing [14:11] jtv: yep, i think you're right. that's a good thing, right? :) [14:11] I can take abel's branch from schwuk [14:12] some good, some bad, like life :) [14:12] we should at least be sure to remove merged branches tho [14:12] sinzui: thanks! [14:12] jtv's technique to to remove the branch from his repo and let the lines in PR go red [14:12] * jtv blushes [14:13] flacoste: what's up with leonard's custom-methods-get branch? [14:13] sinzui: I move them. I keep directories for "waiting for review," "merging," "merged" etc. [14:13] barry: that should have been updated to needs-reply [14:13] barry: it would be back in needs-review today [14:13] flacoste: k, thanks [14:14] * barry thinks stub's branches should be moved to wip [14:14] anything else on the queue? [14:15] 5 [14:15] 4 [14:15] 3 [14:15] 2 [14:15] 1 [14:15] [TOPIC] mentoring update [14:15] New Topic: mentoring update [14:15] I see that only bigjools has signed up for on-call review [14:15] any updates? [14:16] I think it should be mandatory. [14:16] * bigjools is going to join Barry for a few hours my PM on Mondays [14:16] yep, thanks bigjools [14:16] allenap has agreed to do on-call reviews but hasn't updated the wiki [14:16] I think schwuk signed up but I don't remember who with and when [14:16] I'm doing it now :) [14:16] we will start next week [14:17] I've been allocating branches to mentoree's but the on-call review is the best way to get experience. [14:17] allenap: thanks! [14:17] sinzui: i agree. how did the oncalls work while i was away? did we keep up or fall behind? [14:18] I think all was well. I had very little work last Friday in fact. [14:19] sinzui: no end of week 3 freak out? :) [14:19] it was pretty busy at the start of the week but tapered...just as it should [14:19] bac: that's great! [14:20] anything else on mentoring? [14:20] 5 [14:20] 4 [14:20] 3 [14:20] 2 [14:20] 1 [14:20] [TOPIC] review process [14:20] New Topic: review process [14:20] * '''Pre-imp calls are falling by the wayside''' (gmb) [14:21] i'll just paste in gmb's comments since he's not here: [14:21] * '''Pre-imp calls are falling by the wayside''' (gmb). Since we've adopted the on-call review process I've noticed that less and less people (including myself) have been having pre-imp calls before they start work. Partly, I figure that this is because the on-call process allows the developer to talk about their implementation decisions with the reviewer and so helps to substitute for a pre-imp in a lot of ways. However, I've had one [14:21] branch land this cycle that happily had r= from the reviewer but which, had there been a pre-imp, might not have landed in the form it did (which would have avoided me needing to ask for an RC). Should we be enforcing the requirement to have pre-imp calls, or at least be more scrutinous of those branches that don't? [14:21] scrutinous? [14:21] bac: works for me. [14:22] bac: gmb is making up words again [14:22] i see a lot of branches with no pre-imp [14:22] i've noticed the same thing gmb has noticed [14:22] We could start by asking why there was none. [14:22] we usually have on on the foundations team [14:23] i sometimes think that pre-imp calls are not the right emphasis [14:23] i've been thinking about something similar: any branch that gets a needs-reply represents a flaw in the process [14:23] well [14:23] hold your horses barry [14:23] maybe one cause is that there wasn't sufficient pre-impl call [14:24] there are many needs-reply that aren't serious [14:24] I write my notes/pre-imp material like a cover letter and send it to flacoste for discussion. [14:24] what we want is to be 'ready to code', but i've had many cases where there was a pre-imp call, but after starting to work i realised it wasn't actually ready to code [14:24] sinzui: i think that's an /excellent/ idea [14:24] intellectronica: the format or sending it to flacoste? [14:24] i should do this more often, and maybe we should even consider establishing this as a process [14:24] i can testify that sinzui techinque works very well [14:25] I think you can recommend it but make it optional, personally I prefer the phone call [14:25] Fankly I rarely feel something is ready to code until I've written the code—in my head if nowhere else. [14:25] bigjools: we do have a phone [14:25] sinzui: does this pre-impl letter turn into your review cover letter? [14:25] bigjools: after I read the notes [14:25] bigjools: My letter is a start of a phone call and sometime gobby [14:25] bigjools: sure, phone call is great, but it's short, and you can easily miss things [14:25] woa [14:26] actually sinzui is modest here [14:26] writing a note is great for some, sure [14:26] but it won't always work for me [14:26] because his notes often includes high-level version of what will become his tests [14:26] barry: the pre-imp letter has tests and rules, the cover letter has implementation detail. Both would read like a spec [14:27] bigjools: you should see it as the start of practicing TDD :-) [14:27] very interesting [14:27] ha :) [14:27] What I tend to do is to write notes during the phone call [14:27] exactly, we do that for specs, but almost never for bugs, but i noticed that when i do that (for specs) the implementation is much more predictable [14:28] perhaps a better approach would be to establish a pre-imp template? [14:28] an A5! [14:28] a TPS report! [14:28] bigjools: i think that can be useful, like we have for specs [14:28] flacoste: we better add inkscape to lp deps then [14:29] I don't like the idea of a template for pre-imps; that makes you focus too much on what's there, and too little on what makes your branch unique. [14:29] Could we just "review" the cover letter? [14:29] in my book, the template would be very bare [14:29] I think it's more to direct the conversation to make sure you don't miss salient points [14:29] Danilo's been quoting my cover letters in reviews, which is sort of a first step towards reviewing cover letters. [14:30] bigjools: the idea of the pre-impl call is to discuss a design, not to come up with one [14:30] A template that makes sure you don't miss any salient points is likely to look exactly like the review checklist. [14:30] Did I say it was? [14:30] flacoste: right [14:31] bigjools: no, you didn't [14:31] it's just that having on paper the overall plan makes it easier to see if you miss salient points during the convesation [14:31] so, are there any concrete proposals? [14:31] maybe the word "template" is wrong, "guidelines" is better [14:31] example? [14:32] off the top of my head, performance, testing, security, meeting specs [14:33] actually, my best specs, the ones that made it easiest to go ahead with the implementation, mention which files/objects are going to change, how it will be tested, etc'... [14:34] i guess we should move this discussion to the mailing list. i'm not sure there are any action items here [14:34] other than to remind people to have a pre-imp :) [14:34] bigjools: right :) [14:34] [ACTION] barry to remind people to have pre-impl calls [14:34] ACTION received: barry to remind people to have pre-impl calls [14:35] well, that's it for me. does anybody have anything not on the agenda? [14:35] 5 [14:35] 4 [14:35] 13 [14:35] 2 [14:35] bingo!! [14:35] #endmeeting [14:35] Meeting finished at 15:36. [14:35] * jtv is sunstroked today. Never mind jtv. [14:35] thanks everyone! [14:35] thanks barry [14:35] barry: thank you! [14:36] tanks barry === kiko is now known as kiko-fud === salgado is now known as salgado-lunch === kiko-fud is now known as kiko === salgado-lunch is now known as salgado