| === mrevell is now known as mrevell-lunch | ||
| === mrevell-lunch is now known as mrevell | ||
| === salgado-afk is now known as salgado | ||
| jtv | hey barry | 14:57 |
|---|---|---|
| barry | jtv: hi | 14:57 |
| barry | #startmeeting | 15:00 |
| barry | hello everyone and welcome to this week's ameu reviewer's meeting. who's here today? | 15:00 |
| gmb | me | 15:00 |
| jtv | Reviewers meeting? I came here for the Revue Girls Meeting... | 15:00 |
| sinzui | me | 15:00 |
| jtv | me | 15:01 |
| sinzui | jtv: it immediately follows this meeting. | 15:01 |
| jtv | sinzui: ta | 15:01 |
| allenap | me | 15:01 |
| jtv | abentley: "me" :) | 15:01 |
| abentley | me me me | 15:01 |
| bac | me | 15:01 |
| BjornT | me | 15:02 |
| bigjools | me | 15:02 |
| cprov-afk | me | 15:02 |
| barry | danilos: flacoste ping | 15:02 |
| flacoste | me | 15:03 |
| barry | salgado: ping | 15:03 |
| jtv | barry: danilos was going to be here | 15:03 |
| salgado | me! | 15:03 |
| barry | intellectronica sends apologies | 15:03 |
| barry | jtv: thanks | 15:03 |
| barry | i think that's everyone... | 15:03 |
| barry | == Agenda == | 15:04 |
| barry | * Roll call | 15:04 |
| barry | * Next meeting | 15:04 |
| barry | * Action items | 15:04 |
| barry | * Queue status | 15:04 |
| barry | * Mentoring update | 15:04 |
| barry | * Review process | 15:04 |
| barry | * watch for dead end links ([[https://bugs.edge.launchpad.net/launchpad/+bug/254436|bug 254436]] - flacoste) | 15:04 |
| ubottu | Launchpad bug 254436 in launchpad-foundations "Launchpad contains lots of dead-end links" [Undecided,Incomplete] | 15:04 |
| ubottu | Launchpad bug 254436 in launchpad-foundations "Launchpad contains lots of dead-end links" [Undecided,Incomplete] https://launchpad.net/bugs/254436 | 15:04 |
| barry | * extended demo explanation in cover letter (salgado, celso) | 15:04 |
| barry | [TOPIC] next meeting | 15:04 |
| barry | week +=1 ? | 15:04 |
| barry | if you can't make it, please update ReviewerMeetingAgenda with your apologies | 15:04 |
| barry | [TOPIC] * Action items | 15:05 |
| barry | * sinzui to put XXX report into Makefile | 15:06 |
| sinzui | Done. | 15:06 |
| barry | sinzui: awesome, thanks | 15:06 |
| barry | * barry to propose new bug tag for tech debt paydown | 15:06 |
| barry | done | 15:06 |
| * barry remembers that abentley already updated UsingMergeProposals | 15:06 | |
| barry | [TOPIC] * Queue status | 15:07 |
| barry | don't forget to cleanup your PR branches when they land (he says as an offender) | 15:07 |
| barry | only 4 pink branches | 15:07 |
| barry | danilos: any word on stub/launchpad/trivial? | 15:08 |
| danilos | barry: haven't heard from stub, it's needs-reply | 15:08 |
| barry | danilos: ah, the PR status just hasn't been updated then? | 15:08 |
| danilos | barry: that's right | 15:08 |
| barry | danilos: cool, thanks for the update | 15:09 |
| barry | the PR queue doesn't look too bad. does anybody have comments on the review queue? | 15:09 |
| danilos | it looked bad yesterday, thanks to bac for doing most of the work on it | 15:10 |
| * barry applauds bac! | 15:10 | |
| bac | actually it wasn't *that* bad | 15:10 |
| barry | apologies for slacking on monday due to u.s. holiday | 15:10 |
| barry | [TOPIC] * Mentoring update | 15:11 |
| barry | abentley: i think you're the only mentat right now. anything you want to mention? | 15:11 |
| abentley | No, haven't done anything since last week. | 15:12 |
| barry | abentley: cool | 15:12 |
| barry | let me just mention that i think we have slots available, if anybody has ideas for new recruits, please let me know | 15:13 |
| barry | [TOPIC] * Review process | 15:14 |
| barry | * watch for dead end links ([[https://bugs.edge.launchpad.net/launchpad/+bug/254436|bug 254436]] - flacoste) | 15:14 |
| ubottu | Launchpad bug 254436 in launchpad-foundations "Launchpad contains lots of dead-end links" [Undecided,Incomplete] https://launchpad.net/bugs/254436 | 15:14 |
| ubottu | Launchpad bug 254436 in launchpad-foundations "Launchpad contains lots of dead-end links" [Undecided,Incomplete] | 15:14 |
| barry | flacoste: the floor is yours | 15:14 |
| barry | flacoste: ^^ | 15:15 |
| flacoste | that bug describe an issue where we have alot of dead end links | 15:15 |
| flacoste | link to empty pages | 15:15 |
| sinzui | Pages with structure and nothing else? | 15:15 |
| flacoste | so that's something we need to look for in reviews | 15:16 |
| flacoste | UI review ideally | 15:16 |
| barry | flacoste: can you reiterate here what you mentioned in our standup meeting, re: ui reviews? | 15:16 |
| flacoste | sure | 15:17 |
| flacoste | actually, it's more about ui pre-implementation | 15:18 |
| flacoste | so I discussed with beuno last week how he wanted to interact with the team for UI considerations | 15:18 |
| flacoste | i suggested that when we work on a feature involving UI it would be great if he could join the pre-implentation call | 15:19 |
| flacoste | to discuss the UI aspect | 15:19 |
| flacoste | he agreed that it would be a good idea | 15:19 |
| flacoste | so it's something to try out | 15:19 |
| flacoste | so if somebody asks you for a pre-impl and it involves UI | 15:20 |
| flacoste | try to get beuno involved | 15:20 |
| barry | flacoste: +1 ...done? | 15:21 |
| flacoste | yes | 15:21 |
| barry | flacoste: thanks, i'm really glad beuno will be more involved in ui preimps | 15:21 |
| barry | * extended demo explanation in cover letter (salgado, celso) | 15:22 |
| barry | salgado, cprov-afk the floor is yours | 15:22 |
| salgado | celso? | 15:22 |
| salgado | I'm not sure what this is about | 15:22 |
| === cprov-afk is now known as cprov | ||
| salgado | okay, I think I know | 15:22 |
| cprov | right, I know | 15:22 |
| barry | * Salgado and Celso will propose an extended version of the current DemoURL section in our review cover-letter. Most of the times features and/or bugfixes are not properly expressed by a single URL, they involve a sequence of steps (a story) and should be described like that. This way, we expect reviewers (and readers in general) to observe the proposed change in a more holistic aspect, possibly catching details that will result in a | 15:23 |
| barry | more consistent revision. | 15:23 |
| * sinzui often needs to state several URLs and which user to use. | 15:23 | |
| * barry too | 15:23 | |
| * sinzui also needs to remark that there is no URL, check the oops reports or database. | 15:24 | |
| cprov | exactly, most of the times a single URL is not enough to represent an UI change. | 15:24 |
| salgado | I think this is different than a demo URL | 15:24 |
| salgado | I see the demo URL as something to show a new UI you added | 15:24 |
| EdwinGrubbs | me | 15:24 |
| jtv | Maybe a "how to reproduce/demonstrate" section? | 15:24 |
| barry | shouldn't this be captured in a story doctest in the branch? | 15:24 |
| salgado | barry, in general it is | 15:25 |
| barry | e.g. "to demonstrate, read .../story/frobnicate.txt" | 15:25 |
| jtv | But it's nice to have a quick guide to doing it interactively | 15:25 |
| sinzui | This issue relates to how we verify this works on staging. | 15:25 |
| barry | jtv: maybe story doctests should have a summary section at the top? | 15:26 |
| jtv | barry: they should do that already | 15:26 |
| barry | jtv: +1 | 15:26 |
| jtv | barry: but make it too long and it becomes a drag on development | 15:26 |
| barry | jtv: if you've got to write it anyway, put it in the test and keep it short and sweet | 15:27 |
| barry | jtv: followed by detailed user story | 15:27 |
| sinzui | I think the demo section should be transformed into a statement about how to verify the feature/fix in development, staging, production. | 15:27 |
| jtv | Maybe the problem here is that our doctests and pagetest stories are too long to get a good overview. | 15:27 |
| barry | jtv: yes, definitely a problem already identified | 15:27 |
| sinzui | A bug fix is does not necessarily change a user story | 15:28 |
| barry | sinzui: that's different. that's like a q/a plan | 15:28 |
| sinzui | I don't think this id different | 15:28 |
| cprov | sinzui: I like this idea, a guide to reproduce the problem in production and check the fix in development then in staging/edge when it gets committed | 15:28 |
| barry | sinzui: different in the sense: you probably wouldn't put that in our tree | 15:29 |
| sinzui | What is the purpose of providing demo steps if it is not to verify that the code works as expected | 15:29 |
| barry | sinzui: you'd put that in our q/a artifacts | 15:29 |
| sinzui | barry: that is exactly what I am saying | 15:29 |
| barry | sinzui: then we agree! :) | 15:30 |
| sinzui | a bug probably does not have a use story because we captured the story when created the feature | 15:30 |
| barry | sinzui: this is a good idea because we've already said we want two people to do q/a for each issue | 15:30 |
| allenap | sinzui: For UI, say, it gives the reviewer an immediate handle on the end result, then they can review the changes in that light. | 15:30 |
| cprov | barry: i agree it's q/a, the idea is to introduce those artifacts earlier in the development workflow. | 15:30 |
| barry | cprov: +1. so where/how do we capture these? wiki? pastebin? tree? | 15:31 |
| cprov | barry: i'd say it must be in the review cover-letter | 15:32 |
| cprov | review-proposal summary, to be precise. | 15:32 |
| barry | cprov: it's probably not a bad thing to begin to hash out during preimp either | 15:32 |
| barry | iow, during pre-imp ask how you would q/a the change | 15:33 |
| flacoste | i like these ideas | 15:33 |
| flacoste | discussing qa during pre-impl | 15:33 |
| cprov | barry: agreed, it should be considered as part of 'understanding/reproducing the problem' | 15:33 |
| flacoste | and putting the QA validation steps in the cover letter | 15:33 |
| barry | +1 | 15:34 |
| sinzui | +1 | 15:34 |
| * abentley is a bit worried about how much we keep saying "pre-impl" | 15:34 | |
| abentley | In my experience that doesn't happen much. | 15:34 |
| sinzui | abentley: make it happen | 15:34 |
| * sinzui does | 15:34 | |
| * gmb does, too | 15:34 | |
| * abentley lacks a time machine. | 15:34 | |
| * sinzui uses good habits to compensate for poor skilz | 15:35 | |
| barry | salgado, cprov i'd like to capture this in our documentation. can i give one of you the following action: email the list and add it to the wiki? | 15:35 |
| cprov | barry: sure | 15:35 |
| barry | sinzui: do you suggest, as a reviewer, outright rejection of non preimp'd (and q/a planned) branches? | 15:35 |
| barry | cprov: thanks | 15:35 |
| salgado | thanks for taking it, cprov | 15:35 |
| abentley | To be clear, I'm speaking as a reviewer, not a submitter. All *my* stuff has a pre-impl. | 15:35 |
| barry | [ACTION] cprov will email the list and update the wiki, re: review q/a plan during preimp,and include that in the cover letter | 15:36 |
| cprov | salgado: np, I reckon you already have enough in your plate ;) | 15:36 |
| sinzui | barry: I think Rinchen might want that. QA signoff after landing is in our process | 15:36 |
| gmb | abentley: It's not unheard of for a reviewer to say "this is bong; go and have a call with someone about it..." | 15:36 |
| gmb | Though with OCR that seems to happen less, AFAICT | 15:37 |
| abentley | Also, I think that open-sourcing Launchpad will probably reduce the likelihood of pre-impls. | 15:37 |
| barry | gmb: that's been my observation as well, but as a reviewer, i've let it slide (probably too much) | 15:37 |
| barry | abentley: tasty can of worms there | 15:38 |
| gmb | Om nom nom | 15:38 |
| barry | so, should we hard-assed about requiring pre-imps? | 15:38 |
| barry | and q/a plans? | 15:39 |
| gmb | I think, re: pre-imps, it really depends on branch size. | 15:39 |
| jtv | gmb: +1 | 15:39 |
| abentley | barry: I was a guest in the bzr stand-up, and it's clear they think pre-impls won't happen in a real open-source project. | 15:39 |
| flacoste | i think the only thing we should be hard-assed about is rejecting bad branches | 15:39 |
| gmb | For less than, say, 200 lines of diff, a pre-imp might not be necessary (particularly if it's mopstly removals) | 15:39 |
| barry | one thing i'm worried about is possible effect on increasing overhead (and thus discouraging) cleanup branches | 15:39 |
| sinzui | I hate to be a Nazi on this issue, but this issue comes up every month. We need to take some form of action to improve the quality of our branches. | 15:39 |
| flacoste | if pre-impl avoids bad branches, then people will do it | 15:40 |
| gmb | Right. | 15:40 |
| flacoste | because otherwise they'll be rejected in review | 15:40 |
| gmb | Even a mid-imp is better than nothing, because at least someone has said "yes, your approach is sane" | 15:40 |
| gmb | Which isn't the reviewer's first job | 15:40 |
| gmb | Since sanity can be domain specific. | 15:40 |
| flacoste | hmm | 15:41 |
| flacoste | good point | 15:41 |
| * abentley cannot judge sanity of 75% of things submitted to him for review. | 15:41 | |
| gmb | (So, Soyuz makes my brain bleed, but when bigjools says he's discussed a change with cprov I can accept there's some sanity to it) | 15:41 |
| barry | flacoste, sinzui maybe reviewers should be more willing to punt on a branch early if it's had no preimp, lacks a q/a plan, looks like crack | 15:41 |
| flacoste | i think abentley and gmb have a valid points here | 15:41 |
| sinzui | barry: I have done that as a sanity check | 15:41 |
| cprov | gmb: and 'vice-versa' ;) | 15:41 |
| gmb | cprov: No, I trust *you* implicitly ;) | 15:42 |
| barry | abentley: i agree. in mailman, we occasionally have email discussions about approaches, but it's mostly rare | 15:42 |
| * gmb waits for bigjools' boot to descend | 15:42 | |
| cprov | gmb: historically, I've done the most insane changes, you know that :) | 15:42 |
| barry | flacoste: i agree, all good points | 15:42 |
| * bigjools -> winds up | 15:42 | |
| sinzui | I expect someone on my team to accept my tests and implementation plan. I also expect that person to agree with how many branches it will take to complete. | 15:42 |
| barry | sinzui, abentley so preimps are probably best done with team mates, rather than the general lpdev population? | 15:43 |
| barry | then reviewers with less domain knowledge will have more confidence that the approach is sound, and will have a q/a plan in the cover letter to guide them | 15:44 |
| abentley | barry: Yes. And reviews submitted to team mates are less likely to need pre-impl. | 15:44 |
| jtv | barry: definitely, unless you're unsure of your footing in some other important area of your branch. | 15:44 |
| sinzui | barry: often, but I have used BjornT for example when he was person with the most experience with the issue. | 15:44 |
| barry | abentley: i still don't want to give up on reviewers can review anything | 15:44 |
| barry | jtv: yes, you might need multiple pre-imps (i've done that) | 15:45 |
| * sinzui tends to bounce around domains. | 15:45 | |
| barry | sinzui: good point | 15:45 |
| gmb | barry: Reviewers *can* review anything. But, for example, if allenap reviews one of my branches, he doesn't necessarily need me to have had a pre-impl because he can call me on its crack-ness pretty quickly. | 15:45 |
| abentley | barry: I'm not suggesting giving up on that, just noting a corollary. | 15:45 |
| barry | gmb: good point | 15:45 |
| gmb | barry: In which case pre/mid-imp and review become one. | 15:45 |
| flacoste | gmb: well | 15:46 |
| gmb | (Although that's not an excuse to not have had one, I'll grant you) | 15:46 |
| flacoste | the point of the pre-implentation is not only to bounce crack branch at review time | 15:46 |
| * barry apologies for going over 45 minutes, but wants to let this good discussion continue for a bit | 15:46 | |
| flacoste | but to make sure that you don't work on crack branch | 15:46 |
| flacoste | it's about building quality in | 15:46 |
| flacoste | if pre-impl ensures quality in, we should do it more | 15:46 |
| gmb | flacoste: You're right, of course. | 15:46 |
| flacoste | if not, we should drop it as waste | 15:47 |
| barry | flacoste: and also to not waste time | 15:47 |
| barry | reduce waste | 15:47 |
| barry | flacoste: +1 | 15:47 |
| gmb | I think it's definately a positive that we have pre-imps. BjornT has saved me from myself many times during a pre-imp ;) | 15:47 |
| jtv | amen | 15:48 |
| barry | so we've run over. i'd like to carry this forward on the main lp list, so that we can come up with action items next week | 15:48 |
| barry | [ACTION] barry will move the preimp discussion to the ml | 15:48 |
| barry | thank you everyone for some very good points | 15:49 |
| barry | apologies for going long | 15:49 |
| barry | #endmeeting | 15:49 |
| jtv | yay! | 15:49 |
| abentley | Thanks barry | 15:49 |
| jtv | thanks barry | 15:49 |
| === salgado is now known as salgado-lunch | ||
| === ursula_ is now known as Ursinha | ||
| === salgado-lunch is now known as salgado | ||
| === cprov is now known as cprov-afk | ||
| === thumper_laptop is now known as thumper | ||
| === salgado is now known as salgado-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!