[14:57] <jtv> hey barry
[14:57] <barry> jtv: hi
[15:00] <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:01] <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:02] <BjornT> me
[15:02] <bigjools> me
[15:02] <cprov-afk> me
[15:02] <barry> danilos: flacoste ping
[15:03] <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:04] <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] <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:05] <barry> [TOPIC]  * Action items
[15:06] <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:07] <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:08] <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:09] <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:10] <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:11] <barry> [TOPIC]  * Mentoring update
[15:11] <barry> abentley: i think you're the only mentat right now.  anything you want to mention?
[15:12] <abentley> No, haven't done anything since last week.
[15:12] <barry> abentley: cool
[15:13] <barry> let me just mention that i think we have slots available, if anybody has ideas for new recruits, please let me know
[15:14] <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] <barry> flacoste: the floor is yours
[15:15] <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:16] <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:17] <flacoste> sure
[15:18] <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:19] <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:20] <flacoste> so if somebody asks you for a pre-impl and it involves UI
[15:20] <flacoste> try to get beuno involved
[15:21] <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:22] <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] <salgado> okay, I think I know
[15:22] <cprov> right, I know
[15:23] <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:24]  * 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:25] <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:26] <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:27] <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:28] <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:29] <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:30] <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:31] <barry> cprov: +1.  so where/how do we capture these?  wiki?  pastebin?  tree?
[15:32] <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:33] <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:34] <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:35]  * 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:36] <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:37] <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:38] <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:39] <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:40] <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:41] <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:42] <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:43] <barry> sinzui, abentley so preimps are probably best done with team mates, rather than the general lpdev population?
[15:44] <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:45] <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:46] <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:47] <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:48] <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:49] <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