| 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:58 |
| 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 | 08:59 |
| mrevell | Okay, let's assume you're shy :) | 09:00 |
| 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:01 |
| 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:03 |
| mrevell | jamesh: Yeah, if no one says "me" in the roll call I tend not to drag it out. | 09:04 |
| === salgado is now known as salgado-afk | ||
| === salgado-afk is now known as salgado | ||
| === mrevell is now known as mrevell-lunch | ||
| === mrevell-lunch is now known as mrevell | ||
| 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:00 |
| barry | BjornT: cool, thanks | 15:01 |
| bigjools | me | 15:01 |
| jtv | me | 15:01 |
| flacoste | me | 15:01 |
| danilos | me | 15:01 |
| barry | salgado: ? | 15:02 |
| bac | allenap is off today i think | 15:02 |
| salgado | me | 15:02 |
| barry | schwuk: ? | 15:02 |
| schwuk | me | 15:02 |
| schwuk | sorry - was concentrating on another channel | 15:03 |
| gmb | me | 15:03 |
| bigjools | his first Soyuz review probably put him off ;) | 15:03 |
| === barry changed the topic of #launchpad-meeting to: agenda | ||
| schwuk | bigjools: :P | 15:03 |
| gmb | Sorry - I've been having connectivity issues | 15:03 |
| === barry changed the topic of #launchpad-meeting to: Launchpad Meeting Grounds | Channel logs: http://irclogs.ubuntu.com/ | Meeting Logs: Logs available at http://kryten.incognitus.net/mootbot/meetings/ | ||
| 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:03 |
| 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:04 |
| 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:05 |
| 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:06 |
| 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:07 |
| 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:08 |
| 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:09 |
| 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:10 |
| 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:11 |
| 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:12 |
| 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:13 |
| 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:14 |
| 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:15 |
| 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:16 |
| 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:17 |
| 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:18 |
| 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:19 |
| 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:20 |
| 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:21 |
| 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:22 |
| 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:23 |
| 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:24 |
| 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:25 |
| 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:26 | |
| 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:27 |
| * 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:28 |
| 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:29 |
| 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:30 |
| 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 | 15:31 |
| bigjools | <Wayne> Denied </Wayne> | 15:31 |
| 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:32 |
| 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:33 |
| 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:34 |
| 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:35 |
| 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:36 |
| 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:37 |
| 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:38 | |
| 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:39 |
| 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:40 |
| * 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:41 |
| 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:42 |
| 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:43 |
| 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:44 |
| 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:45 |
| 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:46 |
| bac | thanks baw | 15:47 |
| === Edwin is now known as EdwinGrubbs | ||
| === salgado is now known as salgado-lunch | ||
| === salgado-lunch is now known as salgado | ||
| === jamesh_ is now known as jamesh | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!