/srv/irclogs.ubuntu.com/2009/02/04/#launchpad-meeting.txt

=== bigjools-out is now known as bigjools
=== salgado-afk is now known as salgado
=== mrevell is now known as mrevell-lunch
=== mrevell-lunch is now known as mrevell
flacosteme15:01
mars(psst, flacoste, we haven't started yet)15:02
marsas evidenced by barry's absence15:02
barrysorry for the delay folks!15:06
barry#startmeeting15:06
MootBotMeeting started at 09:06. The chair is barry.15:06
MootBotCommands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]15:06
barrywelcome to today's ameu reviewer's meeting, who's here today?15:06
allenapme15:06
marsme15:06
gary_posterme15:06
intellectronicame15:06
bacme15:06
flacosteme15:06
salgadome15:06
intellectronicabarry: abel apologises, he's in transit15:07
barryintellectronica: thanks15:07
abentleyme15:07
BjornTme15:07
barrybigjools, cprov, EdwinGrubbs ping15:08
* bigjools offers apologies as he is sprinting15:08
bigjoolsas is cprov15:08
barrygmb:, rockstar, salgado, sinzui ping15:08
sinzuioops15:08
barrybigjools, cprov no worries!  enjoy the sprint15:08
salgadobarry, me'ed already. :)15:08
bigjoolswe're redesigning the world, it's very enjoyable15:09
barrysalgado: oops, sorry ;}15:09
EdwinGrubbsme15:09
barry[TOPIC] agenda15:09
MootBotNew Topic:  agenda15:09
barry * Roll call15:09
barry * encourage appropriate RENormalizer use in doctests (it is nicer than ellipses in many cases), gary15:09
barry * Action items15:09
barry * Mentoring update15:09
barry * Peanut gallery (anything not on the agenda)15:09
flacostei have a peanut gallery item15:09
abentleybigjools: remember to include beer waterfalls.15:10
gmbme (sprinting, so distracted)15:10
bigjoolsabentley: ++15:10
barryflacoste: cool, i'll make sure we get to it15:10
barry[TOPIC] * encourage appropriate RENormalizer use in doctests (it is nicer than ellipses in many cases), gary15:10
MootBotNew Topic:  * encourage appropriate RENormalizer use in doctests (it is nicer than ellipses in many cases), gary15:10
barrygary_poster: the floor is yours15:10
gary_poster:-) k.  So, I wanted to make sure everyone knew about the RENormalizing in zope.testing15:11
gary_posterAnd I wanted to see if we could encourage folks to use it more when appropriate15:11
gary_posteris anyone not familiar with what it does?15:11
barrygary_poster: can you explain what that is for those not familiar with it?15:11
abentleygary_poster: me15:11
bacme15:11
intellectronicagary_poster: not a clue, tbh15:11
gary_posterk15:11
* BjornT knows about it, and doesn't like it so much :)15:11
gary_posterso, you use it when an ellipsis is too generic, and when an example output might be more helpful15:12
gary_posterso for instance, in your test, you might not want to specify what a host name is15:12
gary_posterso you can write in your test that http://launchpad.net/ should be normalized.15:12
gary_posterIt's not good for big long strings IMO15:12
barrygary_poster: do you have an example?15:13
gary_posterbut good for short bits in which having real text would help15:13
flacostemy main gripes for it is that it's hard to setup using our current infrastructure15:13
abentleygary_poster: What does normalized mean in this context?15:13
intellectronicagary_poster: is there an example we can look at?15:13
rockstarme15:13
gary_posteryeah I was thinking that I should have gotten that.  One sec, we have one in the tree (my fault ;-) )15:13
marsis this a feature already in doctest?15:13
flacosteno15:14
flacosteit's an addition on top of doctest15:14
flacosteyou have to install it as an outputchecker15:14
flacosteiirc15:14
gary_posterright.  you hook it up in tests.py15:14
flacostethat's the setup part that isn't convenient15:14
flacostesince we usually don't have a separate setup for doctests15:14
flacostebut it's probablys omething we should change15:14
marssetuptools extension points FTW15:14
sinzuime15:15
BjornTgary_poster: fwiw, i would like RENormalizing more, if it was possible to use it from within the doctest, instead of doing everything in the harness15:15
gary_posterright.  sorry my grep was too promiscuous15:15
barryflacoste: well, we kind of do, but it's tricky to add stuff globally15:15
* sinzui keeps typing into the wrong window15:15
gary_posterBjornT: I can see that.15:15
barryBjornT: you mean something like a + option?15:15
flacostelike Bjorn says, having to modify the harness everytime you want to use it isn't convenient15:15
marsoh, /that/ kind of setup - not activating the tool, but setting up the harness.  I get it.15:16
BjornTgary_poster, flacoste: i'm not worried about the difficulty of setting up, but rather making it clear to the reader that something special is going on15:16
flacostebarry: +115:16
BjornTbarry: yes, something like15:16
barryflacoste, BjornT that makes a lot of sense (maybe: i still don't quite know what renormalizing looks like :)15:16
intellectronicagary_poster: perhaps it would be better to write about this to the list, so that we can discuss after we've all familiarized ourselves with it?15:16
gary_posterok, so in the set up:15:16
gary_posterlib/canonical/launchpad/mail/ftests/test_stub.py15:16
flacostebarry: think of it as a regex that is applied before comparing the output15:16
barryflacoste: that's kind of cool15:17
flacostebarry: it is!15:17
flacostenot just that convenient to use15:17
flacosteespecially if you need a bunch of different normalizing rules for different examples in the same test15:17
flacostebeing able to control it from within the doctest would be a huge gain15:18
intellectronicaoh, now i understand. i agree with BjornT, having this transformation done behind the scene without any indication in the document itself sounds very confusing to me15:18
barrygary_poster: nice example, i get it15:18
* barry agrees15:18
flacostewell15:18
flacostei think the confusion springs for us to be accustomed to seeing ellipses everywhere15:19
gary_posterso the premise is that it is supposed to make it easier to get the gist; and that it makes the test a-bit-to-a-lot more precise than an ellipsis15:19
flacosteif the policy was never to use ellipses15:19
flacosteit isn't confusing15:19
flacostebecause you know that normalisation is happening15:19
flacostealways15:19
flacosteor can expect it to be15:19
barryi've never looked: how hard is it to add new +OPTIONS?15:20
flacostei think it is very hard15:20
flacostenot pluggable at all15:20
gary_posterso anyway, my proposal is that we encourage the use of this thing.15:20
gary_posterRight15:20
barrydang15:20
marsbarry, doctest.register_optionflag(name)15:20
gary_posterYou may be able to do it in zope.testing15:20
flacosteok, i'm on crack15:20
gary_posterbecause that is already essentially a fork of dictes15:20
gary_posterheh15:20
gary_posterdoctest15:20
gary_postersince it is not pluggable15:21
flacostewell, mars seems to indicate that it is15:21
marshttp://docs.python.org/library/doctest.html#doctest.register_optionflag15:21
MootBotLINK received:  http://docs.python.org/library/doctest.html#doctest.register_optionflag15:21
abentleybzr development frequently uses assertContainsRe in its unittests, and it's quite useful.15:21
flacosteheh MootBot, you're not on strike today!15:21
barryum, okay, that registers the flag, how do you tell it what the flag should do?15:21
abentley(It asserts that a string contains a given regex)15:21
gary_posterWell, I know that customizing doctest has been a problem15:21
gary_posterand right15:21
gary_posterthis is not just a flag15:22
abentleyThis seems similarly useful.15:22
abentleyThough it does kinda get away from the idea that the tests are documentation.15:22
barrygary_poster: are you interested in investigating whether renormalization can be hooked into a +flag?  if so, then i think we could start using it in a limited/experimental way15:22
flacostebarry: i don't think we should start there15:22
flacostei think we can try using it today15:23
flacostewith the current limitations15:23
gary_posterabentley: do you feel that the example I gave took it away from documentation?15:23
flacostewhich will give us more exposure to the setup problems15:23
gary_posterthe idea there was that it helped, since an example revision # is more informative than an ellipsis15:23
abentleygary_poster: No.  The example you gave didn't seem to need it either, though.15:23
barryflacoste: that sounds good to me15:23
gary_posterabentley: you mean, an ellipsis would have been as good or better for you?15:24
abentleyRegexes tend to look like line noise.15:24
abentleygary_poster: Yes.15:24
gary_posterabentley: hm.  diff'rent strokes and all I guess.15:24
barryabentley: but you don't generally see the regexp in the doctest, and also ellipses can often be distracting15:24
gary_posterok so anyway, barry, that's my spiel. :-)15:25
gary_posteras an aside, maybe we ought to investigate manuel or other more flexible doctest mechanisms at some point, but that's a much bigger deal15:25
barrygary_poster: thanks.  can you send an email to the ml explaining this and letting them know we can use it experimentally?15:25
abentleygary_poster: Okay, I skimmed too quickly.15:25
barrygary_poster: yeah, and yeah :)15:25
abentleygary_poster: It's not as bad as I thought.15:26
gary_posterabentley: cool15:26
gary_posterbarry: will do.15:26
barry[ACTION] gary_poster to send an email to ml explaining RENormalization15:26
MootBotACTION received:  gary_poster to send an email to ml explaining RENormalization15:26
barrythanks15:26
flacostemy item?15:26
barryflacoste: sure15:26
barry[TOPIC] flacoste peanut gallery item15:26
flacosteit's about dead zone reviews15:26
MootBotNew Topic:  flacoste peanut gallery item15:26
flacosteTOPC: dead zone reviews15:26
barryflacoste: you mean reviews outside my cell phone range?15:27
flacostehow to proceed with branches that are ready when nobody is on call15:27
flacostefor example, stuart put out a branch last week for review15:27
flacostethat hasn't been picked up by anybody yet15:27
flacostesimply because he didn't try finding a reviewer15:28
barryrockstar: when are we getting mp queues? :)15:28
flacostenobody was on call when the branch was reviewed15:28
rockstarbarry, dunno.  thumper talked about it.15:28
barryflacoste: didn't try or couldn't find?15:28
flacostei think the former, but nobody was on call15:29
flacosteand previously15:29
rockstarbarry, but there is http://edge.launchpad.net/launchpad/+requestedreviews15:29
flacostethe general queue was assigned automatically15:29
flacostebut this kind of died with the advent of OCR15:29
barryflacoste: well, not so much automatically, but i get what you're saying15:29
rockstarflacoste, I still think it's the reviewee's responsibility to find a reviewer, even if there are tools for the reviewer to find outstanding reviews.15:29
flacosteright, you put a branch on pending reviews and somebody would get to it15:29
barryflacoste: as a fallback, you can always ping someone on #lp-code.  i've done reviews that way several times15:29
allenaprockstar: Do you mean +merges? That link breaks.15:30
barryrockstar: yep15:30
rockstarallenap, no, +requestedreviews15:30
barryflacoste: or, set up a mp and pick some reviewer at random to review it15:30
flacostewell15:30
flacostethat would be annoying for everybody i think15:30
barryflacoste: yeah, but it shouldn't happen often, right?15:31
allenaprockstar: Ah yeah, I see it.15:31
rockstarallenap, sorry, address is https://edge.launchpad.net/~launchpad/+requestedreviews15:31
gmbflacoste: can't we just make it policy that the OCR checks the queue at the start of their shift for any un-owned branches?15:31
flacostethat's more what I was getting into15:31
flacosteback in the days of PendingReviews15:31
flacosteallocating reviews was part of the OCR shift15:31
flacosteand this was never replaced when we switched to MPs15:31
gmbflacoste: Also15:31
gmbA simple option would be to put your nick and a link to the mp in the queue in #lp-r15:32
gmbRegardless of whether someone is on call.15:32
gmbThat way the next OCR knows what needs dealing with.15:32
flacosteIRC is a lossy medium15:32
rockstarI re-he-ally don't like the idea of just throwing a review on a pile and saying "get to this"15:32
flacosteso i wouldn't count on it15:32
gmbrockstar: What, any more than we do already? The only difference now is that hte reviewer can say "sorry, branch X took longer than I expected, no can do sunny jim"15:33
rockstargmb, but there's still an personal interaction there.15:33
gmbTrue15:34
rockstarFor instance, I think there's real value in someone ASKING me if I can do a review even if I'm OCR.15:34
flacosteok, so we are changing the policy here15:34
flacosteif we agreees on this change15:34
rockstarCode reviews should not be mechanical.15:34
rockstarMaybe we're changing the policy to match our current flow.15:34
flacostewell15:34
barryrockstar: i tend to agree with you15:34
flacostewhat about when we get drive-by contributors?15:34
flacostewe'll have to have some form of queue there15:35
rockstarMy understanding has always been that it's reviewee's responsibility to make sure the branch gets reviewed.15:35
flacosterockstar: it is15:35
flacostebut15:35
flacostethere were mechanism to make that easy15:35
flacostenow, it's much more "personal"15:35
rockstarYes, it's called "hey flacoste, can you review my branch" ?15:35
BjornTrockstar: the problem is that it's not always easy to find someone online. if there's a policy that if you submitted the merge request yesterday, you can jump the queue, then it might work. otherwise we'll have branches that take a long time to get reviews, just because you'r not at irc at the right time15:35
flacosteexactly15:35
flacosteespecially in dead zones like thailand15:36
barryflacoste: the main thing i want to ensure tho, is that it's foolproof that the queue is managed correctly.  meaning specifically it cannot be possible that someone forgot to pop an item off the queue and an ocr wastes time reviewing something already reviewed because they failed to look in the magical right place15:36
rockstarBjornT, you can email.  I've done reviews for jtv that way before.15:36
flacostewhich increase the turn-around time even more15:36
flacostesince there is no queue management right now15:37
rockstarIf you're requesting a review from someone on the other side of the world, you're not worried about turnaround time.15:37
bacemail increases turn-around time?  not if stub/whoever looks at the schedule and picks the next OCR to come on duty15:37
flacostei think it's ok for the policy to be, you have to find someone to agree to review your branch15:37
flacostegiven the current number of reviewers it shouldn't be a problem15:37
flacostebut we need to make that official policy15:37
flacosteit means that once you have a merge proposal out there15:38
flacostethere should be an indivual assigned to it15:38
flacostenot just the LP team15:38
flacosteotherwise, your branch isn't going to get reviewed15:38
flacostethat's how review allocation is done through Launchpad15:38
rockstarflacoste, well, it can be the LP team, just in case that person doesn't get to it in time (like in gmb's case).15:38
flacostethat doesn't work15:38
rockstarIn that case, you ask someone else and they pick it up instead.15:39
marsflacoste, rockstar, would adding a "Pending review by" column to +activereviews help?15:39
flacosteand you assign it to them15:39
flacosteif it's assigned to the team15:39
barryflacoste, or anyone: do we know of other people having problems or is it primarily stub?  if the latter, we can add special cases for his situation15:39
flacosteit's assigned to no one15:39
gary_posterbarry: foolproof that queue is managed correctly: sometimes you get a branch that has not been assigned specifically for you to review.  Maybe in that case you make a "i'm reviewing it" review initially as a marker for others?15:39
rockstarmars, I don't know if it would.  It wouldn't help me.15:39
marsrockstar, ok15:39
barrydammit!15:40
rockstarIt would be helpful if you could un-request someone to review it.15:40
flacostebarry: i only know of stub, but that's mainly because he's used to the old scheme15:40
barrywhat did i miss? ;)15:40
barryrockstar: that would be useful.  then stub could pick the next schedule ocr, which would work 90% of the time15:40
rockstarBecause ideally, once every reviewer says "Approve" then the status of the BMP should be "approved"15:41
barryflacoste: as an ocr, i'd be happy if there were a stub branch or two that were waiting for me when i started15:41
abentleygary_poster: Would it make sense for "claim review" to be separate from "perform review"?15:41
flacosteyes!15:41
flacostethere is no way to claim a review15:41
gary_posterabentley: I think that would be cool15:41
flacosteexcept by doing it15:41
barryflacoste: i'd just pick them off first.  the question is, how best to seed the queue?15:42
flacosteor marking your vote as abstain15:42
flacostewhich sounds weird15:42
barryflacoste: what about this as a trial: stub can pick the next scheduled ocr, assign the mp to him, and leave a note in the channel topic15:42
barryflacoste: that sounds simple and i think it would work for me.  if that fails then we can try something else15:43
flacostebarry: i don't think making an exception for stub is useful at this point15:43
flacostewe just have to state that you have to find a reviewer for your branch15:43
flacosteso if there is no indivual reviewer assigned to it15:43
flacosteyour branch is not going to be reviewed15:44
flacosteand run with it for a while15:44
barryflacoste: stub was just an example.  i think most devs don't fall into dead zones15:44
flacosteit's possible that it's not a problem in practice for anybody to be able to find somebody to commit to a review15:44
bacreviewers will need to get in the habit of checking their +requestedreviews ... i know i've never looked at it before15:44
flacostethe problem here is that he was expecting somebody to pick it up, like it used to work15:44
rockstarbac, it's a relatively new view.15:44
flacostethe problem is that there was a subtle change in workflow without any formal announcement15:44
flacostebac: well, if you commit to something, you commited to something15:45
barryflacoste: i think i get what you're saying15:45
bacflacoste: well, i didn't commit to it if someone just assigns the review to me15:45
flacosteyou only need to look at +requestedreviews if you have trouble remembering your commitments :-)15:45
flacostebac: nobody would15:45
flacostethey would ask you permission to assign you to it15:45
bacthat's not what barry suggested ^^, which seems reasonable to me15:46
barryflacoste: it's always been dev's responsibility to get reviewed, but you're right that it's relatively new that you had to actively round up a reviewer15:46
flacostehey bac, can you review this branch for me (because you are OCR, or simply because you are cool)15:46
flacostesure!15:46
flacostethen I assign the review to you15:46
bacflacoste: that's great if it is the flow we decide on.15:46
barryflacoste: also would work: an email saying "hey barry, you're on call tomorrow so i left you a present"15:47
flacostewell, it's the one that we seem to be using and that is possible given the current tool15:47
flacoste(since we don't want to manage a queue somewhere else)15:47
bacah, nm.  being assigned a review will generate email addressed directly to me, so i'd know about it.15:47
barrybac: yeah15:48
* bac still like the new +requestedreviews view...15:48
rockstarbac, yeah, I do too, but I think it's important to have that acknowledgement/handshake in the review process.15:48
barryflacoste: we're out of time today.  can you bring this up on the ml just to solidify the policy?  or email me to hash it out first15:49
BjornTthe problem with this workflow is that now you suddenly rely on a single person to have time to review your branch, instead of relying that any of the OCRs have time15:49
bacQ:  i see ~launchpad/+requestedreviews has very few listed, which means ~launchpad is not automatically being added to some new MPs.  why is that?15:49
bacBjornT: only if you're not around for the OCR.15:49
abentleybac: Most likely because reviews created by email don't have that set.15:50
BjornTbac: or the branch is around too late, so that you have to wait the next OCRs to start their shift15:50
flacostebarry: i'll start a thread15:50
barrybac: there are also some cases where mp's won't auto-assign a team15:50
bacabentley: i created one this morning using lpsend15:50
barryflacoste: thanks15:51
flacosteBjornT: that's what I usually do, if i'm too late, i'll just ask somebody the next day15:51
abentleybac: and it automatically requested ~launchpad to review?15:51
barry[ACTION] flacoste to take dead zone reviews discussion to ml15:51
MootBotACTION received:  flacoste to take dead zone reviews discussion to ml15:51
bacabentley: it did not.  i expected it would.15:51
bacabentley: wrong assumption?15:51
BjornTflacoste: right. it would still be nicer to ask a group of people, instead of a single person.15:51
abentleybac: Wrong assumption.15:51
abentleybac: Not that it shouldn't, but it doesn't.15:52
bacabentley: ok.15:52
flacosteBjornT: yeah, but my experience is that this doesn't really work in practice15:52
flacosteat least, that's my experience with the advent of OCR15:52
flacostesure, the OCR might swap it between themselve15:52
flacostebut only because the first one agreed to it15:52
BjornTflacoste: that's why i said "would" :) it doesn't work atm, but it would be nice if it would15:53
flacostei agree15:53
barryokay, sorry to do this and my apologies for starting the meeting late, but we're way over, so let's end things here and take it to the ml or #lp-code15:53
barry#endmeeting15:53
MootBotMeeting finished at 09:53.15:53
barrythanks everybody15:53
=== mrevell is now known as mrevell-dinner
=== salgado is now known as salgado-afk

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!