[08:58] <mrevell> #startmeeting
[08:58] <MootBot> Meeting started at 08:58. The chair is mrevell.
[08:58] <MootBot> Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]
[08:58] <mrevell> Welcome to February's Launchpad users meeting!
[08:59] <mrevell> [TOPIC] Roll call
[08:59] <MootBot> New Topic:  Roll call
[08:59] <mrevell> Let's get an idea of who's here for the meeting. If you're here to take part, say me.
[08:59] <mrevell> me
[09:00] <mrevell> Okay, let's assume you're shy :)
[09:01] <mrevell> We'll move straight onto user questions, as it appears there aren't many people here for the meeting
[09:01] <mrevell> [TOPIC] User questions
[09:01] <MootBot> New Topic:  User questions
[09:01] <mrevell> We don't have any user questions on the agenda, so if you have a question or suggestion or comment for the Launchpad team, go ahead!
[09:03] <mrevell> okay, if you do have a question or comment for the Launchpad team, you can find someone from the team just about 24 hours a day in #launchpad. Otherwise mail feedback@launchpad.net.
[09:03] <mrevell> #endmeeting
[09:03] <MootBot> Meeting finished at 09:03.
[09:03] <jamesh> mrevell: that was short
[09:03] <hexmode> k
[09:04] <mrevell> jamesh: Yeah, if no one says "me" in the roll call I tend not to drag it out.
[15:00] <statik> me
[15:00] <barry> #startmeeting
[15:00] <MootBot> Meeting started at 15:00. The chair is barry.
[15:00] <MootBot> Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]
[15:00] <barry> hello everyone and welcome to this week's ameu reviewer's meeting
[15:00] <barry> who's here today?
[15:00] <BjornT> me
[15:00] <intellectronica> me
[15:00] <statik> me
[15:00] <bac> me
[15:00] <BjornT> gmb has problems with his internet connection atm
[15:01] <barry> BjornT: cool, thanks
[15:01] <bigjools> me
[15:01] <jtv> me
[15:01] <flacoste> me
[15:01] <danilos> me
[15:02] <barry> salgado: ?
[15:02] <bac> allenap is off today i think
[15:02] <salgado> me
[15:02] <barry> schwuk: ?
[15:02] <schwuk> me
[15:03] <schwuk> sorry - was concentrating on another channel
[15:03] <gmb> me
[15:03] <bigjools> his first Soyuz review probably put him off ;)
[15:03] <schwuk> bigjools: :P
[15:03] <gmb> Sorry - I've been having connectivity issues
[15:03] <barry> [TOPIC] agenda
[15:03] <MootBot> New Topic:  agenda
[15:03]  * barry needs more coffee
[15:03] <barry> == Agenda ==
[15:03] <barry>  * Roll call
[15:03] <barry>  * Next meeting
[15:03] <barry>  * Action items
[15:03] <barry>  * Queue status
[15:03] <barry>  * Mentoring update
[15:03] <barry>  * Review process
[15:03] <barry>    * sabdfl request: make sure enums to be in the same interface files as the schemas where they are expressed.
[15:04] <barry>    * thumper - unwrapping import statements (line wrapping that is)
[15:04] <barry> [TOPIC] next meeting
[15:04] <MootBot> New Topic:  next meeting
[15:04] <barry> same time and place?  anyone know they won't be here?
[15:04]  * bigjools will be sprinting
[15:04] <bigjools> but I might make it
[15:05] <barry> k, thanks
[15:05] <bac> sprinting too
[15:05]  * jtv same
[15:05] <BjornT> quite a lot of people will be sprinting next week
[15:05] <bigjools> good point
[15:05] <barry> which sprint is it?
[15:05] <salgado> team lead sprint?
[15:05] <bigjools> Team Leads and Soyuz
[15:05] <salgado> oh, soyuz too?
[15:05] <bigjools> yarp
[15:05] <barry> should we still have a meeting then?
[15:06] <bigjools> Soyuz Guy #3 (tm) is starting next week
[15:06] <flacoste> and bzr
[15:06] <flacoste> not that we have any bzr people in this meeting
[15:06] <sinzui> me me me
[15:07] <barry> flacoste: right.  i think mwhudson is the only guy who's gonna be at the next asiapac meeting ;)
[15:07] <barry> well, i guess we'll still have the ameu meeting.  if nobody shows up, it'll be blissfully short :)
[15:07] <barry> [TOPIC]  * Action items
[15:07] <MootBot> New Topic:   * Action items
[15:08] <barry>  * (continued) intellectronica to put cover letter draft on wiki
[15:08] <barry> i think this was done, right?
[15:08] <intellectronica> barry: yup
[15:08] <bigjools> url?
[15:08] <intellectronica> https://launchpad.canonical.com/ReviewRequestTemplate
[15:08] <barry> intellectronica: awesome!  can you email the link?
[15:08] <bigjools> can we link off PR to that too - I have enough bookmarks already :)
[15:09] <barry> bigjools: +1
[15:09] <barry> intellectronica: do you mind adding that link and emailing it to the launchpad list?
[15:09] <intellectronica> barry: sure, i'll do that
[15:09] <barry> thanks!
[15:10] <flacoste> shouldn't sumit-review
[15:10] <flacoste> modified to use that template?
[15:10] <barry> [ACTION] intellectronica to email link to new review template and add a link to PR
[15:10] <MootBot> ACTION received:  intellectronica to email link to new review template and add a link to PR
[15:10] <barry> flacoste: let's get some feedback on it, but then, probably so
[15:10]  * bigjools just added it to PR
[15:10] <barry> great, thanks!
[15:11] <barry>  * (continued) sinzui to look into outputting PR stanza by default in `review-submit`
[15:11] <sinzui> I submitted my change to automatically output the PR block. I wait for mwhudson's approval.
[15:11] <barry> sinzui: rock on!
[15:11] <barry> [TOPIC]  * Queue status
[15:11] <MootBot> New Topic:   * Queue status
[15:12] <barry> general queue is clear
[15:12] <barry> and only 3 needs-review branches in the pink
[15:12] <barry> queue looks pretty good, any comments?
[15:12] <intellectronica> the general queue is clear, but we've had to reject a branch of jml's
[15:13] <barry> intellectronica: ah yes, i see it.  has jml been notified?
[15:13] <intellectronica> i think something went wrong with his submissions, since there are quite a few of them on the mailing list with no wiki entries
[15:13] <intellectronica> yes, danilo and i wrote to him
[15:13] <barry> intellectronica: great, thanks
[15:14] <barry> any other comments about the queue?
[15:14] <barry> 5
[15:14] <barry> 4
[15:14] <barry> 3
[15:14] <barry> 2
[15:14] <barry> 1
[15:14] <barry> [TOPIC]  * Mentoring update
[15:14] <MootBot> New Topic:   * Mentoring update
[15:15] <barry> nothing from me here...
[15:15] <barry> 5
[15:15] <sinzui> I think schwuk needs to get more reivews. I think he has had only two since he started.
[15:15] <bac> mentoring with gavin is going well, but slow.  please allocate him some branches if you clear out the general queue.
[15:16] <barry> sinzui, bac good points, thanks
[15:16] <bac> sinzui: i think gavin has done two also
[15:16] <barry> so we should favor mentorees when assigning general queue branches?
[15:16] <sinzui> +1
[15:16] <bac> +1
[15:16] <intellectronica> +1
[15:16] <intellectronica> but does it happen often enough?
[15:16] <schwuk> sinzui: I'm going to try going on call this Friday, so I should get more then.
[15:17] <sinzui> schwuk: Fab
[15:17] <bigjools> as long as we mentorees don't get bombarded with branches
[15:17] <bac> if gavin does any reviews next week i may need to ask one of you to mentor it, since i'll be sprinting.
[15:17] <barry> [AGREED] favor (but don't overwhelm) mentorees when assigning general queue branches
[15:17] <MootBot> AGREED received:  favor (but don't overwhelm) mentorees when assigning general queue branches
[15:17] <gmb> bac: I'm happy to mentor if needs be.
[15:17] <intellectronica> bigjools: unlikely. it really doesn't happen very often these days
[15:17] <bac> gmb: thanks
[15:18] <bigjools> intellectronica: true - most of them are done on-call
[15:18] <barry> i'll need someone to help mentor schwuk during pycon, but that's not for a few weeks yet
[15:18] <barry> er, sorry, bigjools
[15:19] <bigjools> we can sort that out nearer the time
[15:19] <barry> yep!
[15:19] <barry> anything else?
[15:19] <barry> 5
[15:19] <barry> 4
[15:19] <barry> 3
[15:19] <barry> 2
[15:19] <barry> 1
[15:19] <barry> [TOPIC]  * Review process
[15:19] <MootBot> New Topic:   * Review process
[15:20] <barry>    * sabdfl request: make sure enums to be in the same interface files as the schemas where they are expressed.
[15:20] <barry> so mark was pretty clear that we reviewers need to be more aware of where interfaces are defined
[15:21] <barry> i moved an interface from person.py to teammembership.py because it was in the wrong place
[15:21] <barry> so he just wanted me to communicate to you to keep an eye out for these things while reviewing code
[15:21] <barry> (over)
[15:22] <sinzui> barry: That reminds me that there was never an announcement that ISchemas can be defined in browser.*
[15:22] <barry>    * thumper - unwrapping import statements (line wrapping that is)
[15:22] <bac> barry: did you mention it to the developer who put it in person originally?
[15:22] <barry> bac: i did not
[15:22] <barry> bac: i think mark's point was more that reviewers need to be on guard for this
[15:23] <bac> barry: true, but it might have been a good "teachable moment."
[15:23] <barry> sinzui: i don't remember that decision specifically
[15:23] <flacoste> sinzui: it's still status quo on this i think
[15:23] <barry> bac: true
[15:24] <flacoste> barry: i'm probably the one at fault here, since I took care of the move of all generic launchpad ones
[15:24] <flacoste> barry: i probably put it person.py to avoid a cyclic import problem
[15:24] <flacoste> or maybe i'm just fabulating
[15:24] <salgado> or maybe it was me
[15:24] <sinzui> bzr annotate
[15:24] <barry> flacoste: no it makes some sense, because it was actually /used/ in person.py
[15:25] <barry> but it was easy to avoid the circular import
[15:25] <barry> sinzui: if it's worth it (i don't think it is)
[15:25] <sinzui> I don't think it is worth it
[15:25] <barry> or if you're curious :)
[15:26] <barry> anything else on this topic?
[15:26] <barry> ...
[15:26] <barry>    * thumper - unwrapping import statements (line wrapping that is)
[15:26] <bigjools> a big hairy +1 from me for that
[15:26] <barry> so in the meeting agenda thumper suggests putting multi-line imports each on a separate line
[15:26]  * barry hasn't actually talked to him about htis
[15:27] <barry> it occurs to me too that __all__ has the same issue
[15:27] <flacoste> i think this is crack
[15:27] <salgado> -1
[15:27] <salgado> that's crack
[15:27] <flacoste> this is to avoid conflicts
[15:27] <intellectronica> it's a nice idea, but the risk is that when merging we'll get too many imports that aren't being used
[15:27] <flacoste> but these are trivial conflicts
[15:27] <barry> bug salgado said he didn't like this in a recent review of mine
[15:27] <flacoste> live with it
[15:27] <intellectronica> also, these conflicts are easy to solve
[15:27] <danilos> -0
[15:27] <bigjools> it's not the conflicts that are the problem
[15:27] <flacoste> and it uses a lot of screen estate
[15:27] <bigjools> it's the diff
[15:27] <salgado> I can live with that in the __all__, but not in imports
[15:28]  * barry agrees with salgado
[15:28] <bigjools> working out wtf changed when 4 lines are reformatted
[15:28] <gmb> bigjools: using meld will solve that one.
[15:28] <gmb> (or something similar)
[15:28] <flacoste> who cares about these changes?
[15:28] <flacoste> we have make lint to see if they are spurious
[15:28] <salgado> bigjools, make lint should tell you if the changes are correct or not
[15:28] <bigjools> fair point
[15:29] <barry> [VOTE] one __all__ entry per-line but not per multiline import
[15:29] <MootBot> Please vote on:  one __all__ entry per-line but not per multiline import.
[15:29] <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:29] <MootBot> E.g. /msg MootBot +1 #launchpad-meeting
[15:29] <flacoste> -1
[15:29] <MootBot> -1 received from flacoste. 0 for, 1 against. 0 have abstained. Count is now -1
[15:29] <salgado> -1
[15:29] <MootBot> -1 received from salgado. 0 for, 2 against. 0 have abstained. Count is now -2
[15:29] <gmb> -1
[15:29] <MootBot> -1 received from gmb. 0 for, 3 against. 0 have abstained. Count is now -3
[15:29] <BjornT> i don't care much either way, as long as we're consistent. if we're going to change the coding style, we should change all imports to conform with it. i don't thing that's worth the trouble
[15:29] <sinzui> -1
[15:29] <MootBot> -1 received from sinzui. 0 for, 4 against. 0 have abstained. Count is now -4
[15:30] <bac> -1
[15:30] <MootBot> -1 received from bac. 0 for, 5 against. 0 have abstained. Count is now -5
[15:30] <BjornT> -1
[15:30] <MootBot> -1 received from BjornT. 0 for, 6 against. 0 have abstained. Count is now -6
[15:30] <bigjools> +0
[15:30] <MootBot> Abstention received from bigjools. 0 for, 6 against. 1 have abstained. Count is now -6
[15:30] <barry> okay, i think you all misunderstood the sense of my question, but we'll just say "i know what you mean" :)
[15:30] <barry> the -1's are /against/ one-line per import
[15:30] <barry> so...
[15:30] <barry> -1
[15:30] <MootBot> -1 received from barry. 0 for, 7 against. 1 have abstained. Count is now -7
[15:30] <intellectronica> -1
[15:31] <MootBot> -1 received from intellectronica. 0 for, 8 against. 1 have abstained. Count is now -8
[15:31] <schwuk> -1
[15:31] <MootBot> -1 received from schwuk. 0 for, 9 against. 1 have abstained. Count is now -9
[15:31] <barry> [AGREED] thumper's suggestion of one-line per import is rejected
[15:31] <MootBot> AGREED received:  thumper's suggestion of one-line per import is rejected
[15:31] <barry> [TOPIC] free-for-all
[15:31] <MootBot> Vote is in progress. Finishing now.
[15:31] <MootBot> Final result is 0 for, 9 against. 1 abstained. Total: -9
[15:31] <MootBot> New Topic:  free-for-all
 Denied </Wayne>
[15:32] <barry> i'm done, anybody have anything not on the agenda?
[15:32] <salgado> nope
[15:32] <bac> one quick thing
[15:32] <sinzui> I'd like to suggest that when large branches are submitted, they are looked over for how they can b broken into smaller branches
[15:33] <jtv> sinzui: shouldn't that happen before?
[15:33]  * barry looks sheepish
[15:33] <sinzui> I had a looks a one of celso's branches, and saw four separate deliverables.
[15:33] <bac> i noticed 'rocketfuel-branch' no longer updates trunk.  this led to a much larger than expected diff i generated to review.  just mentioning
[15:34] <bigjools> it's down to the dev to manage this - if the review doesn't like it the branch should be rejected.  The rules have been well announced.
[15:34] <bigjools> s/review/reviewer/
[15:34] <BjornT> jtv: it should, yes. but people will submit large branches anyway.
[15:34] <barry> bigjools: +1
[15:34] <sinzui> bigjools: True, but the chances of getting parts reviewed and landed is better than a giant branch
[15:35] <BjornT> for big branches, i think the reviewer should have to explain why the branch couldn't be split up
[15:35] <bigjools> sinzui: that's why we have the 800 line rule though isn't it?
[15:35] <sinzui> bigjools: I was able to review two parts of celso's large branch and he landed them before you and schwuk got the backend and frontend pieces
[15:35] <schwuk> sinzui: what about the time expended helping get the branch split? That's taken from other reviews.
[15:35] <gmb> Maybe review-submit should wc -l the diff and warn the dev before they submit something oversized.
[15:36] <sinzui> schwuk: I spent 10 minutes to find the parts
[15:36] <BjornT> schwuk: i think the idea is that the reviewer will learn how to split up branches, so he won't do the same mistake in the future.
[15:36] <flacoste> sinzui: if it makes it that future branch are smaller that's good
[15:36] <sinzui> My core point is that during the preimp, the bug/spec needs to break the branches down to sane sizes
[15:37] <bigjools> branch size is something I try to stay aware of during the branch's whole lifetime so I am not surprised when I submit it
[15:37] <bigjools> sinzui: +1
[15:37] <sinzui> I think in celso's case, he could have delivered the parts through out the week instead of on one day
[15:38] <bigjools> pre-imp should discuss this as part of the "ready to code" criteria
[15:38]  * sinzui 's branch in pending-reviews is special. there are no code or test changes.
[15:39] <flacoste> and
[15:39] <flacoste> i'd like to recall a point
[15:39] <flacoste> big branches needs an arrangement with a reviewer
[15:39] <flacoste> *before* they are submitted
[15:39] <flacoste> i think we should just reject branches over 800k as a matter of policy
[15:39] <flacoste> 800 lines
[15:39] <gmb> flacoste: +1
[15:40] <gmb> Especially for oncall work.
[15:40] <flacoste> and leave the dev to find a reviewer
[15:40] <LarstiQ> 800k lines or 800 lines?
[15:40] <sinzui> So 800+ in the general queue is forbidden?
[15:40] <bigjools> +1
[15:40] <gmb> It dies up a reviewer for far too long.
[15:40] <barry> flacoste: i would support that (even as a recent offender) IF lpreview complained bitterly about > 800 line diffs
[15:40] <gmb> s/dies/ties
[15:40] <bigjools> gmb: it can kill them too :)
[15:40] <flacoste> larger than 800 lines is ok
[15:40] <gmb> barry: It should be fairly easy to patch lpreview to do that.
[15:40] <barry> let there be a --isuck flag
[15:40] <flacoste> if you have a reviwerer that commited to it
[15:40]  * gmb is looking at the source now.
[15:40] <flacoste> this has been a policy for some time now
[15:40] <flacoste> but we didn't really enforce it
[15:40] <flacoste> now is probably time to do it
[15:41]  * sinzui branches lpreview for 800 line slap test
[15:41] <bigjools> so > 800 with prior agreement with a reviewer only?
[15:41] <flacoste> lpreview will only submit a 800 lines patch if the -r option is used
[15:41] <flacoste> yes
[15:41] <gmb> sinzui: lpreview/__init__.py:313 would be an ideal place for the slap code.
[15:41] <flacoste> bigjools: yes
[15:41] <bigjools> flacoste: sounds good to me
[15:41] <barry> sinzui or gmb: who wants the action item on this one? hack lpreview to enforce (with manual override) 800 line limit?
[15:42] <flacoste> idea being that the reviewer will be able to help split the branch earlier
[15:42] <gmb> barry: I'll take it.
[15:42] <barry> gmb: thanks!
[15:42] <flacoste> barry: no manual override
[15:42] <flacoste> the manual override is the -r option
[15:42] <sinzui> no manual
[15:42] <barry> [ACTION] gmb to hack lpreview to enforce 800 line limit
[15:42] <MootBot> ACTION received:  gmb to hack lpreview to enforce 800 line limit
[15:42] <flacoste> -r being the reviewer who agreed to review the branch
[15:42] <barry> flacoste: +1
[15:42] <BjornT> flacoste: using -r has a disadvantage, though.
[15:42] <sinzui> if the review is not specified and the diff is > 800 lines, die
[15:43] <schwuk> should lpreview also have a -mentor argument?
[15:43] <flacoste> BjornT: what is it?
[15:43] <schwuk> or at least a -cc
[15:43] <BjornT> it's not uncommon to ask a reviewer for a review, without telling him how big the diff is
[15:43] <gmb> schwuk: Yes. I think there's abug for that already.,
[15:43] <flacoste> lol
[15:43] <flacoste> BjornT: right!
[15:43] <flacoste> BjornT: what's your suggestion?
[15:43] <BjornT> flacoste: manual override :)
[15:43] <barry> BjornT: --isuck
[15:44] <BjornT> this shouldn't happen often, so supplying an extra argument shouldn't be a problem
[15:44] <sinzui> I think the implicit meaning of -r should suffice
[15:45] <barry> sinzui: i think BjornT has a point though: i might add -r without being warned about branch size
[15:45] <bigjools> I regularly use -r to direct the mail to the reviewer though
[15:45] <bac> i think even if -r is specified there should be an explicit acknowledgment the branch is too big
[15:45] <gmb> sinzui: Ah, but people use that when subitting stuff for PR...
[15:45] <BjornT> sinzui: how often do you ask how big the diff is before accepting to review a branch?
[15:45] <barry> i'd rather, first get the warning
[15:45] <bigjools> you need a new option
[15:45] <gmb> bac: +1
[15:45] <barry> okay, we're out of time for this meeing!  gmb it's in your court now
[15:45] <gmb> "You're submitting a 1200 line diff for review. Don't be silly now. Are you sure? [y/N]"
[15:45] <gmb> barry: Okay.
[15:45] <barry> #endmeeting
[15:45] <MootBot> Meeting finished at 15:45.
[15:46] <barry> thanks everyone!
[15:46] <BjornT> sinzui: branches over 800 should not be common, so no need to optimize for that case. a bit extra work shouldn't be a problem.
[15:46] <bigjools> thanks barry
[15:46] <schwuk> thanks barry
[15:47] <bac> thanks baw