[15:01] <barry> #startmeeting
[15:01] <MootBot> Meeting started at 09:05. The chair is barry.
[15:01] <MootBot> Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]
[15:01] <barry> hello everyone and welcome to this week's ameu reviewer's meeting.  who's here today?
[15:01] <allenap> me
[15:01] <gmb> me
[15:01] <jtv> me
[15:01] <bac> me
[15:01] <sinzui> me
[15:01] <BjornT> me
[15:02] <intellectronica> me
[15:02] <EdwinGrubbs> me
[15:02] <bigjools> me
[15:02] <bigjools> cprov is at debconf
[15:02] <gmb> splitter
[15:02] <bigjools> curveball
[15:03] <barry> danilos: ping
[15:03] <danilos> me
[15:03] <barry> jtv: ping?
[15:03] <jtv> barry: on phone
[15:03] <barry> jtv: np!  hi!
[15:03] <jtv> hi
[15:04] <salgado> me
[15:04] <abentley> me
[15:04] <barry> cool, let's start
[15:04] <barry> [TOPIC] agenda
[15:04] <MootBot> New Topic:  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>    * merge proposals update
[15:04] <barry> [TOPIC] next meeting
[15:04] <MootBot> New Topic:  next meeting
[15:05] <barry> week +=1 ?  anybody know they won't be here?
[15:05] <abentley> I won't be here.
[15:05]  * danilos , for a change, will be around in the next few months :)
[15:05] <barry> abentley: cool, danilos :)
[15:06] <barry> [TOPIC] action items
[15:06] <MootBot> New Topic:  action items
[15:06] <barry> intellectronica: any status on your three? :)
[15:06] <intellectronica> sorry, haven't touched my action items yet. busy times :(
[15:07] <barry> intellectronica: understood.  we'll carry these forward
[15:07] <barry> [TOPIC] queue status
[15:07] <MootBot> New Topic:  queue status
[15:07] <barry> first, thanks everybody for taking on those extra reviews and clearing our queue.  it was greatly appreciated
[15:08] <barry> i think we actually don't look too bad right now, though there are 4 pink branches
[15:08] <barry> notice testsuite2 is no longer at the top of the heap
[15:09] <barry> any comments on the queue from y'all?
[15:09] <sinzui> Are the three top items the ones that were assigned?
[15:10] <barry> sinzui: good question.  i'm pretty sure jml is aware of the mpt branch
[15:10] <barry> abentley: what's up with tim's karma branch?
[15:10] <barry> stub's got a branch for abentley and bigjools has a branch for adeuring
[15:10] <sinzui> I think they are...I confess that I worked on my assigned branch during my on-call day
[15:10] <abentley> barry: He replied to my review last night.  I'll be re-reviewing today.
[15:11] <bigjools> yeah I did abel's today
[15:11] <barry> abentley: cool.  i still don't think i saw that branch to mentor it.  maybe it got stuck in my junk folder or something.  i will look around
[15:11] <barry> bigjools: thanks
[15:12] <abentley> barry: You're not cc'ed, but launchpad-reviews is.
[15:12] <barry> abentley: okay, i'll search my folder
[15:12] <barry> thanks
[15:13] <barry> anything else on the queue?
[15:13] <barry> [TOPIC] mentoring
[15:13] <MootBot> New Topic:  mentoring
[15:13] <barry> nothing much to report here from me.  open to comments from the floor
[15:14] <abentley> I think that people who are being mentored should be referred to as "mentats" :-)
[15:14] <EdwinGrubbs> or mentos
[15:14] <barry> abentley: :)  i just watched dune on hbo a few nights ago :)
[15:15] <barry> okay cool, moving on
[15:15] <barry> [TOPIC] review process
[15:15] <MootBot> New Topic:  review process
[15:15] <barry>    * merge proposals update
[15:16] <barry> bac: thanks for guinea pigging it and emailing your experiences
[15:16] <bac> np. hope it was helpful.
[15:16] <barry> definitely
[15:16] <sinzui> Thou shall know a mentat bug the colour of his lips
[15:17] <abentley> I regret the confusion between review votes and actually approving/disapproving a merge proposal.
[15:17] <barry> anybody else planning on trying it out this week?
[15:17] <gmb> barry: I'll see how loaded I am tomorrow.
[15:17] <barry> gmb: cool
[15:17] <abentley> Some projects, e.g. bazaar, require more than one review.
[15:17] <barry> abentley: do you have any thoughts on changes to make that easier to understand?
[15:18] <abentley> Not yet, but I'm open to ideas.
[15:18] <bac> abentley: we need to update the wiki page to add in the step about doing the actual approval.  the last step mentioned is still 'vote approve'.
[15:19] <barry> abentley: what do you think about being able to establish automatic rules, e.g. two approvals and no rejections == Approve
[15:19] <abentley> bac: Okay.
[15:20] <barry> abentley: you'll update the wiki?
[15:20] <abentley> barry: I will update it.
[15:20] <barry> abentley: thanks
[15:20] <abentley> barry: I think automatic rules are nice.  See: Bundle Buggy.
[15:20]  * barry should have known that bb already has it :)
[15:21] <abentley> Code Reviews are not wholly my design; sabdfl and thumper also had significant input.
[15:22] <abentley> sabdfl didn't want per-project customizations, IIRC.
[15:22] <sinzui> I agree
[15:22] <barry> abentley: i'm still not sure i agree with that, but we can continue discussing it on the ml
[15:23] <barry> i have nothing else on the agenda, so we can continue to discuss this or call it a day
[15:23] <BjornT> if we don't want per-project customizations, wouldn't it be better for merge proposals not to have a status of their own, and instead display the number of votes?
[15:24] <abentley> I should mention that I've started work on supporting "bzr send" for launchpad.  So that should land sometime in August.
[15:25] <barry> BjornT: interesting idea
[15:25] <barry> abentley: would that be hookable so we can replace review-submit, or structures so r-s could wrap it?
[15:26] <abentley> Right now, I'm only looking at changing LP, not bzr.  But yes, r-s could send merge directives today if mwhudson decided to implement that.
[15:27] <barry> abentley: cool
[15:28] <barry> anything else?
[15:28] <abentley> Is now a good time to discuss diff trimming?
[15:28] <barry> abentley: sure
[15:28] <barry> abentley: floor is yours
[15:29] <abentley> So I think it makes sense to trim diffs when sending a review, instead of just inserting comments in the middle of the diff.
[15:29] <abentley> It makes the review more readable, and makes it harder for the reviewee to miss anything you've said.
[15:30] <abentley> Do people agree, disagree, not care?
[15:30] <sinzui> I don't know what you mean by trim. sorry
[15:30] <intellectronica> abentley: i have  no idea what you mean by 'trimming'
[15:30] <intellectronica> sinzui: high five
[15:30] <bac> i definitely disagree for mentats.  the mentor needs to see even the stuff the mentor deemed uninteresting.
[15:31] <abentley> trim: Remove the sections of the diff that you don't want to comment on.
[15:31] <barry> bac: i agree
[15:31] <abentley> bac: The mentor has a mail client that supports threading, one hopes?
[15:31] <barry> bac: er, with you.  mentats should not trim
[15:31] <bac> bac:  yes
[15:31] <bigjools> I think it's down to the branch being reviewed
[15:31] <intellectronica> abentley: i think that's not a good idea, because lots of people other than the reviewer look at those emails and want to see the full diff
[15:31] <barry> abentley: yes, but that's much less convenient for mentoring
[15:31] <bac> abentley: sure, but if i'm mentoring i don't want to have to compare the review with the original, threads or not.
[15:32] <abentley> intellectronica: Why wouldn't they look at the full diff when it was submitted, then?
[15:32] <sinzui> I prefer trimming. When I mentor, I read the entire change set after the mentoree has brought the review to merge-conditional*
[15:32] <barry> fwiw, i never trim the original review, though i do trim follow ups
[15:32] <gmb> Me too.
[15:32] <bigjools> barry: +1
[15:32] <intellectronica> abentley: oh, you mean in replies ...
[15:33] <bac> also as a developer i don't think it's that easy to miss comments in a properly formated review.
[15:33] <abentley> intellectronica: I mean if you want to look at the full diff, read it when it is submitted for review.
[15:33] <gmb> Trimming replies I have no problem with.
[15:33] <barry> also, i find that as i'm reviewing, i'll often want to go back to and comment on an earlier section.  if i trim as i'm reviewing, that's much harder to do, so i just don't trim
[15:33] <intellectronica> still, i think most mail clients format this conveniently enough for the reader
[15:33] <abentley> intellectronica: I do not mean replies, except in the sense that the initial review is a reply to the request for review.
[15:34] <intellectronica> abentley: yes, i mean reply in that sense. i sometimes do reviews like this, by replying to the request and leaving comments interleaved with the original diff
[15:34] <BjornT> i always trim the diffs when i do a review. that's the last thing i do, though, so there isn't a problem going back commenting on other sections
[15:35] <barry> abentley: i sympathize with the readability issue.  it's definitely mua-specific though.  for me, mail.app makes it trivially easy to find the comments.  claws is more difficult
[15:35]  * sinzui ponders how an app that supports block annotation and folding would work
[15:36] <intellectronica> finally, it's more work which i won't really benefit from, so -1 from me on having a policy that requires that :)
[15:36] <abentley> intellectronica: So when you do an initial review, you remove sections of the diff that you don't want to comment on?
[15:36] <intellectronica> abentley: almost never
[15:36] <sinzui> I too backtrack when I review. a comment may relate to 3 parts of the diff and It would be nice to select the comment to see all three chunks
[15:36] <bac> i only remove the most mundane sections
[15:36] <abentley> intellectronica: I think you may have misread me, then.
[15:37] <intellectronica> oh
[15:37] <barry> have we had complaints from devs about unreadable reviews, or in your experience, do devs often miss comments?
[15:38] <barry> i'm trying to understand if this is in response to an existing problem
[15:38] <abentley> intellectronica: I mean that in an initial review, and in replies, it makes sense to me to remove unnecessary sections of the diff.
[15:39] <abentley> barry: No.  I just consider it good protocol, on any ML, to only quote sections that I want to reply to.
[15:39] <BjornT> abentley: +1
[15:40] <barry> i think then a reasonable policy would be to leave it up to the reviewer, with mentats deferring to what works best for their mentor
[15:40] <intellectronica> abentley: right, so i did get you, and as i said, i wouldn't benefit from having this done by others, and it would be more work for me, so -1 on a policy that requires doing that
[15:40] <bigjools> I think it should be down to the reviewer's discretion
[15:40] <bac> barry: +1
[15:40] <abentley> intellectronica: Are you okay with a policy that *permits* it?
[15:40] <bac> IOW, status quo
[15:40] <intellectronica> abentley: sure
[15:41] <abentley> Because right now, policy *forbids* it, IIUC.
[15:41] <bigjools> only for mentees
[15:42] <abentley> barry: I'm happy with the policy you proposed.
[15:43] <barry> [VOTE] allow reviewers to trim initial reviews at their desecration, mentats defer to mentor choice.  +1 agree, -1 disagree
[15:43] <MootBot> Please vote on:  allow reviewers to trim initial reviews at their desecration, mentats defer to mentor choice.  +1 agree, -1 disagree.
[15:43] <MootBot> Public votes can be registered by saying +1/-1/+0 in the channel, private votes by messaging the channel followed by +1/-1/+0  to MootBot
[15:43] <MootBot> E.g. /msg MootBot +1 #launchpad-meeting
[15:43] <barry> +1
[15:43] <MootBot> +1 received from barry. 1 for, 0 against. 0 have abstained. Count is now 1
[15:44] <BjornT> +1
[15:44] <MootBot> +1 received from BjornT. 2 for, 0 against. 0 have abstained. Count is now 2
[15:44] <bigjools> +1
[15:44] <MootBot> +1 received from bigjools. 3 for, 0 against. 0 have abstained. Count is now 3
[15:44] <abentley> +1
[15:44] <MootBot> +1 received from abentley. 4 for, 0 against. 0 have abstained. Count is now 4
[15:44] <gmb> +1
[15:44] <MootBot> +1 received from gmb. 5 for, 0 against. 0 have abstained. Count is now 5
[15:44] <bac> +1
[15:44] <MootBot> +1 received from bac. 6 for, 0 against. 0 have abstained. Count is now 6
[15:44] <allenap> +
[15:44] <sinzui> +1
[15:44] <MootBot> +1 received from sinzui. 7 for, 0 against. 0 have abstained. Count is now 7
[15:44] <allenap> +1
[15:44] <MootBot> +1 received from allenap. 8 for, 0 against. 0 have abstained. Count is now 8
[15:44] <EdwinGrubbs> +1
[15:44] <MootBot> +1 received from EdwinGrubbs. 9 for, 0 against. 0 have abstained. Count is now 9
[15:44] <intellectronica> +1
[15:44] <MootBot> +1 received from intellectronica. 10 for, 0 against. 0 have abstained. Count is now 10
[15:45] <barry> [AGREED] allow trims of initial reviews at reviewer discretion, mentats to follow mentor preferences
[15:45] <MootBot> AGREED received:  allow trims of initial reviews at reviewer discretion, mentats to follow mentor preferences
[15:45] <barry> we're out of time.  thanks for bringing this up abentley, so...
[15:45] <barry> #endmeeting
[15:45] <MootBot> Vote is in progress. Finishing now.
[15:45] <MootBot> Final result is 10 for, 0 against. 0 abstained. Total: 10
[15:45] <MootBot> Meeting finished at 09:49.
[15:45] <barry> thanks everyone, have a good day
[15:45] <bigjools> cheerio!
[15:45] <intellectronica> thanks barry
[15:45] <abentley> Thanks.
[15:45] <gmb> Ta barry.