ReviewerMeeting20090204

Not logged in - Log In / Register

ReviewerMeeting20090204

summary

logs

Feb 04 10:06:18 <barry> #startmeeting
Feb 04 10:06:19 <MootBot>       Meeting started at 09:06. The chair is barry.
Feb 04 10:06:19 <MootBot>       Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]
Feb 04 10:06:38 <barry> welcome to today's ameu reviewer's meeting, who's here today?
Feb 04 10:06:40 <allenap>       me
Feb 04 10:06:41 <intellectronica>       me
Feb 04 10:06:41 <mars>  me
Feb 04 10:06:41 <gary_poster>   me
Feb 04 10:06:44 <bac>   me
Feb 04 10:06:45 <flacoste>      me
Feb 04 10:06:55 <salgado>       me
Feb 04 10:07:05 <intellectronica>       barry: abel apologises, he's in transit
Feb 04 10:07:12 <barry> intellectronica: thanks
Feb 04 10:07:14 <abentley>      me
Feb 04 10:07:39 <BjornT>        me
Feb 04 10:08:02 <barry> bigjools, cprov, EdwinGrubbs ping
Feb 04 10:08:22 *       bigjools offers apologies as he is sprinting
Feb 04 10:08:27 <bigjools>      as is cprov
Feb 04 10:08:37 <barry> gmb:, rockstar, salgado, sinzui ping
Feb 04 10:08:45 <sinzui>        oops
Feb 04 10:08:49 <barry> bigjools, cprov no worries!  enjoy the sprint
Feb 04 10:08:55 <salgado>       barry, me'ed already. :)
Feb 04 10:09:02 <bigjools>      we're redesigning the world, it's very enjoyable
Feb 04 10:09:04 <barry> salgado: oops, sorry ;}
Feb 04 10:09:22 <EdwinGrubbs>   me
Feb 04 10:09:32 <barry> [TOPIC] agenda
Feb 04 10:09:33 <MootBot>       New Topic:  agenda
Feb 04 10:09:40 <barry>  * Roll call
Feb 04 10:09:40 <barry>  * encourage appropriate RENormalizer use in doctests (it is nicer than ellipses in many cases), gary
Feb 04 10:09:40 <barry>  * Action items
Feb 04 10:09:40 <barry>  * Mentoring update
Feb 04 10:09:40 <barry>  * Peanut gallery (anything not on the agenda)
Feb 04 10:09:56 <flacoste>      i have a peanut gallery item
Feb 04 10:10:03 <abentley>      bigjools: remember to include beer waterfalls.
Feb 04 10:10:10 <gmb>   me (sprinting, so distracted)
Feb 04 10:10:13 <bigjools>      abentley: ++
Feb 04 10:10:16 <barry> flacoste: cool, i'll make sure we get to it
Feb 04 10:10:26 <barry> [TOPIC] * encourage appropriate RENormalizer use in doctests (it is nicer than ellipses in many cases), gary
Feb 04 10:10:26 <MootBot>       New Topic:  * encourage appropriate RENormalizer use in doctests (it is nicer than ellipses in many cases), gary
Feb 04 10:10:35 <barry> gary_poster: the floor is yours
Feb 04 10:11:06 <gary_poster>   :-) k.  So, I wanted to make sure everyone knew about the RENormalizing in zope.testing
Feb 04 10:11:22 <gary_poster>   And I wanted to see if we could encourage folks to use it more when appropriate
Feb 04 10:11:30 <gary_poster>   is anyone not familiar with what it does?
Feb 04 10:11:38 <barry> gary_poster: can you explain what that is for those not familiar with it?
Feb 04 10:11:38 <abentley>      gary_poster: me
Feb 04 10:11:40 <bac>   me
Feb 04 10:11:41 <intellectronica>       gary_poster: not a clue, tbh
Feb 04 10:11:42 <gary_poster>   k
Feb 04 10:11:47 *       BjornT knows about it, and doesn't like it so much :)
Feb 04 10:12:09 <gary_poster>   so, you use it when an ellipsis is too generic, and when an example output might be more helpful
Feb 04 10:12:24 <gary_poster>   so for instance, in your test, you might not want to specify what a host name is
Feb 04 10:12:47 <gary_poster>   so you can write in your test that http://launchpad.net/ should be normalized.
Feb 04 10:12:57 <gary_poster>   It's not good for big long strings IMO
Feb 04 10:13:09 <barry> gary_poster: do you have an example?
Feb 04 10:13:14 <gary_poster>   but good for short bits in which having real text would help
Feb 04 10:13:15 <flacoste>      my main gripes for it is that it's hard to setup using our current infrastructure
Feb 04 10:13:17 <abentley>      gary_poster: What does normalized mean in this context?
Feb 04 10:13:17 <intellectronica>       gary_poster: is there an example we can look at?
Feb 04 10:13:31 <rockstar>      me
Feb 04 10:13:36 <gary_poster>   yeah I was thinking that I should have gotten that.  One sec, we have one in the tree (my fault ;-) )
Feb 04 10:13:58 <mars>  is this a feature already in doctest?
Feb 04 10:14:09 <flacoste>      no
Feb 04 10:14:14 <flacoste>      it's an addition on top of doctest
Feb 04 10:14:25 <flacoste>      you have to install it as an outputchecker
Feb 04 10:14:26 <flacoste>      iirc
Feb 04 10:14:35 <gary_poster>   right.  you hook it up in tests.py
Feb 04 10:14:39 <flacoste>      that's the setup part that isn't convenient
Feb 04 10:14:52 <flacoste>      since we usually don't have a separate setup for doctests
Feb 04 10:14:57 <flacoste>      but it's probablys omething we should change
Feb 04 10:14:58 <mars>  setuptools extension points FTW
Feb 04 10:15:06 <sinzui>        me
Feb 04 10:15:11 <BjornT>        gary_poster: fwiw, i would like RENormalizing more, if it was possible to use it from within the doctest, instead of doing everything in the harness
Feb 04 10:15:13 <gary_poster>   right.  sorry my grep was too promiscuous
Feb 04 10:15:17 <barry> flacoste: well, we kind of do, but it's tricky to add stuff globally
Feb 04 10:15:28 *       sinzui keeps typing into the wrong window
Feb 04 10:15:39 <gary_poster>   BjornT: I can see that.
Feb 04 10:15:55 <barry> BjornT: you mean something like a + option?
Feb 04 10:15:56 <flacoste>      like Bjorn says, having to modify the harness everytime you want to use it isn't convenient
Feb 04 10:16:02 <mars>  oh, /that/ kind of setup - not activating the tool, but setting up the harness.  I get it.
Feb 04 10:16:08 <BjornT>        gary_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 on
Feb 04 10:16:09 <flacoste>      barry: +1
Feb 04 10:16:20 <BjornT>        barry: yes, something like
Feb 04 10:16:40 <barry> flacoste, BjornT that makes a lot of sense (maybe: i still don't quite know what renormalizing looks like :)
Feb 04 10:16:47 <intellectronica>       gary_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?
Feb 04 10:16:50 <gary_poster>   ok, so in the set up:
Feb 04 10:16:51 <gary_poster>   lib/canonical/launchpad/mail/ftests/test_stub.py
Feb 04 10:16:56 <flacoste>      barry: think of it as a regex that is applied before comparing the output
Feb 04 10:17:13 <barry> flacoste: that's kind of cool
Feb 04 10:17:17 <flacoste>      barry: it is!
Feb 04 10:17:27 <flacoste>      not just that convenient to use
Feb 04 10:17:41 <flacoste>      especially if you need a bunch of different normalizing rules for different examples in the same test
Feb 04 10:18:02 <flacoste>      being able to control it from within the doctest would be a huge gain
Feb 04 10:18:05 <intellectronica>       oh, 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 me
Feb 04 10:18:08 <barry> gary_poster: nice example, i get it
Feb 04 10:18:26 *       barry agrees
Feb 04 10:18:41 <flacoste>      well
Feb 04 10:19:01 <flacoste>      i think the confusion springs for us to be accustomed to seeing ellipses everywhere
Feb 04 10:19:08 <gary_poster>   so 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 ellipsis
Feb 04 10:19:24 <flacoste>      if the policy was never to use ellipses
Feb 04 10:19:34 <flacoste>      it isn't confusing
Feb 04 10:19:42 <flacoste>      because you know that normalisation is happening
Feb 04 10:19:44 <flacoste>      always
Feb 04 10:19:48 <flacoste>      or can expect it to be
Feb 04 10:20:15 <barry> i've never looked: how hard is it to add new +OPTIONS?
Feb 04 10:20:22 <flacoste>      i think it is very hard
Feb 04 10:20:26 <flacoste>      not pluggable at all
Feb 04 10:20:30 <gary_poster>   so anyway, my proposal is that we encourage the use of this thing.
Feb 04 10:20:31 <gary_poster>   Right
Feb 04 10:20:35 <barry> dang
Feb 04 10:20:38 <mars>  barry, doctest.register_optionflag(name)
Feb 04 10:20:40 <gary_poster>   You may be able to do it in zope.testing
Feb 04 10:20:48 <flacoste>      ok, i'm on crack
Feb 04 10:20:49 <gary_poster>   because that is already essentially a fork of dictes
Feb 04 10:20:51 <gary_poster>   heh
Feb 04 10:20:56 <gary_poster>   doctest
Feb 04 10:21:07 <gary_poster>   since it is not pluggable
Feb 04 10:21:16 <flacoste>      well, mars seems to indicate that it is
Feb 04 10:21:27 <mars>  http://docs.python.org/library/doctest.html#doctest.register_optionflag
Feb 04 10:21:27 <MootBot>       LINK received:  http://docs.python.org/library/doctest.html#doctest.register_optionflag
Feb 04 10:21:36 <abentley>      bzr development frequently uses assertContainsRe in its unittests, and it's quite useful.
Feb 04 10:21:41 <flacoste>      heh MootBot, you're not on strike today!
Feb 04 10:21:46 <barry> um, okay, that registers the flag, how do you tell it what the flag should do?
Feb 04 10:21:55 <abentley>      (It asserts that a string contains a given regex)
Feb 04 10:21:56 <gary_poster>   Well, I know that customizing doctest has been a problem
Feb 04 10:21:57 <gary_poster>   and right
Feb 04 10:22:02 <gary_poster>   this is not just a flag
Feb 04 10:22:20 <abentley>      This seems similarly useful.
Feb 04 10:22:39 <abentley>      Though it does kinda get away from the idea that the tests are documentation.
Feb 04 10:22:44 <barry> gary_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 way
Feb 04 10:22:55 <flacoste>      barry: i don't think we should start there
Feb 04 10:23:05 <flacoste>      i think we can try using it today
Feb 04 10:23:08 <flacoste>      with the current limitations
Feb 04 10:23:09 <gary_poster>   abentley: do you feel that the example I gave took it away from documentation?
Feb 04 10:23:24 <flacoste>      which will give us more exposure to the setup problems
Feb 04 10:23:38 <gary_poster>   the idea there was that it helped, since an example revision # is more informative than an ellipsis
Feb 04 10:23:45 <abentley>      gary_poster: No.  The example you gave didn't seem to need it either, though.
Feb 04 10:23:45 <barry> flacoste: that sounds good to me
Feb 04 10:24:15 <gary_poster>   abentley: you mean, an ellipsis would have been as good or better for you?
Feb 04 10:24:17 <abentley>      Regexes tend to look like line noise.
Feb 04 10:24:27 <abentley>      gary_poster: Yes.
Feb 04 10:24:41 <gary_poster>   abentley: hm.  diff'rent strokes and all I guess.
Feb 04 10:24:53 <barry> abentley: but you don't generally see the regexp in the doctest, and also ellipses can often be distracting
Feb 04 10:25:04 <gary_poster>   ok so anyway, barry, that's my spiel. :-)
Feb 04 10:25:36 <gary_poster>   as an aside, maybe we ought to investigate manuel or other more flexible doctest mechanisms at some point, but that's a much bigger deal
Feb 04 10:25:49 <barry> gary_poster: thanks.  can you send an email to the ml explaining this and letting them know we can use it experimentally?
Feb 04 10:25:51 <abentley>      gary_poster: Okay, I skimmed too quickly.
Feb 04 10:25:58 <barry> gary_poster: yeah, and yeah :)
Feb 04 10:26:00 <abentley>      gary_poster: It's not as bad as I thought.
Feb 04 10:26:04 <gary_poster>   abentley: cool
Feb 04 10:26:11 <gary_poster>   barry: will do.
Feb 04 10:26:24 <barry> [ACTION] gary_poster to send an email to ml explaining RENormalization
Feb 04 10:26:24 <MootBot>       ACTION received:  gary_poster to send an email to ml explaining RENormalization
Feb 04 10:26:27 <barry> thanks
Feb 04 10:26:36 <flacoste>      my item?
Feb 04 10:26:42 <barry> flacoste: sure
Feb 04 10:26:52 <barry> [TOPIC] flacoste peanut gallery item
Feb 04 10:26:52 <flacoste>      it's about dead zone reviews
Feb 04 10:26:53 <MootBot>       New Topic:  flacoste peanut gallery item
Feb 04 10:26:59 <flacoste>      TOPC: dead zone reviews
Feb 04 10:27:19 <barry> flacoste: you mean reviews outside my cell phone range?
Feb 04 10:27:26 <flacoste>      how to proceed with branches that are ready when nobody is on call
Feb 04 10:27:50 <flacoste>      for example, stuart put out a branch last week for review
Feb 04 10:27:58 <flacoste>      that hasn't been picked up by anybody yet
Feb 04 10:28:11 <flacoste>      simply because he didn't try finding a reviewer
Feb 04 10:28:18 <barry> rockstar: when are we getting mp queues? :)
Feb 04 10:28:33 <flacoste>      nobody was on call when the branch was reviewed
Feb 04 10:28:35 <rockstar>      barry, dunno.  thumper talked about it.
Feb 04 10:28:44 <barry> flacoste: didn't try or couldn't find?
Feb 04 10:29:04 <flacoste>      i think the former, but nobody was on call
Feb 04 10:29:07 <flacoste>      and previously
Feb 04 10:29:11 <rockstar>      barry, but there is http://edge.launchpad.net/launchpad/+requestedreviews
Feb 04 10:29:14 <flacoste>      the general queue was assigned automatically
Feb 04 10:29:21 <flacoste>      but this kind of died with the advent of OCR
Feb 04 10:29:34 <barry> flacoste: well, not so much automatically, but i get what you're saying
Feb 04 10:29:56 <rockstar>      flacoste, 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.
Feb 04 10:29:58 <flacoste>      right, you put a branch on pending reviews and somebody would get to it
Feb 04 10:29:59 <barry> flacoste: as a fallback, you can always ping someone on #lp-code.  i've done reviews that way several times
Feb 04 10:30:13 <allenap>       rockstar: Do you mean +merges? That link breaks.
Feb 04 10:30:15 <barry> rockstar: yep
Feb 04 10:30:31 <rockstar>      allenap, no, +requestedreviews
Feb 04 10:30:37 <barry> flacoste: or, set up a mp and pick some reviewer at random to review it
Feb 04 10:30:45 <flacoste>      well
Feb 04 10:30:52 <flacoste>      that would be annoying for everybody i think
Feb 04 10:31:06 <barry> flacoste: yeah, but it shouldn't happen often, right?
Feb 04 10:31:10 <allenap>       rockstar: Ah yeah, I see it.
Feb 04 10:31:13 <rockstar>      allenap, sorry, address is https://edge.launchpad.net/~launchpad/+requestedreviews
Feb 04 10:31:15 <gmb>   flacoste: can't we just make it policy that the OCR checks the queue at the start of their shift for any un-owned branches?
Feb 04 10:31:26 <flacoste>      that's more what I was getting into
Feb 04 10:31:33 <flacoste>      back in the days of PendingReviews
Feb 04 10:31:42 <flacoste>      allocating reviews was part of the OCR shift
Feb 04 10:31:53 <flacoste>      and this was never replaced when we switched to MPs
Feb 04 10:31:58 <gmb>   flacoste: Also
Feb 04 10:32:16 <gmb>   A simple option would be to put your nick and a link to the mp in the queue in #lp-r
Feb 04 10:32:23 <gmb>   Regardless of whether someone is on call.
Feb 04 10:32:31 <gmb>   That way the next OCR knows what needs dealing with.
Feb 04 10:32:32 <flacoste>      IRC is a lossy medium
Feb 04 10:32:38 <rockstar>      I re-he-ally don't like the idea of just throwing a review on a pile and saying "get to this"
Feb 04 10:32:47 <flacoste>      so i wouldn't count on it
Feb 04 10:33:26 <gmb>   rockstar: 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"
Feb 04 10:33:44 <rockstar>      gmb, but there's still an personal interaction there.
Feb 04 10:34:05 <gmb>   True
Feb 04 10:34:11 <rockstar>      For instance, I think there's real value in someone ASKING me if I can do a review even if I'm OCR.
Feb 04 10:34:14 <flacoste>      ok, so we are changing the policy here
Feb 04 10:34:26 <flacoste>      if we agreees on this change
Feb 04 10:34:29 <rockstar>      Code reviews should not be mechanical.
Feb 04 10:34:38 <rockstar>      Maybe we're changing the policy to match our current flow.
Feb 04 10:34:39 <flacoste>      well
Feb 04 10:34:39 <barry> rockstar: i tend to agree with you
Feb 04 10:34:55 <flacoste>      what about when we get drive-by contributors?
Feb 04 10:35:01 <flacoste>      we'll have to have some form of queue there
Feb 04 10:35:02 <rockstar>      My understanding has always been that it's reviewee's responsibility to make sure the branch gets reviewed.
Feb 04 10:35:11 <flacoste>      rockstar: it is
Feb 04 10:35:11 <flacoste>      but
Feb 04 10:35:19 <flacoste>      there were mechanism to make that easy
Feb 04 10:35:33 <flacoste>      now, it's much more "personal"
Feb 04 10:35:37 <rockstar>      Yes, it's called "hey flacoste, can you review my branch" ?
Feb 04 10:35:41 <BjornT>        rockstar: 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 time
Feb 04 10:35:52 <flacoste>      exactly
Feb 04 10:36:02 <flacoste>      especially in dead zones like thailand
Feb 04 10:36:11 <barry> flacoste: 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 place
Feb 04 10:36:17 <rockstar>      BjornT, you can email.  I've done reviews for jtv that way before.
Feb 04 10:36:38 <flacoste>      which increase the turn-around time even more
Feb 04 10:37:16 <flacoste>      since there is no queue management right now
Feb 04 10:37:16 <rockstar>      If you're requesting a review from someone on the other side of the world, you're not worried about turnaround time.
Feb 04 10:37:25 <bac>   email increases turn-around time?  not if stub/whoever looks at the schedule and picks the next OCR to come on duty
Feb 04 10:37:37 <flacoste>      i think it's ok for the policy to be, you have to find someone to agree to review your branch
Feb 04 10:37:49 <flacoste>      given the current number of reviewers it shouldn't be a problem
Feb 04 10:37:57 <flacoste>      but we need to make that official policy
Feb 04 10:38:04 <flacoste>      it means that once you have a merge proposal out there
Feb 04 10:38:10 <flacoste>      there should be an indivual assigned to it
Feb 04 10:38:15 <flacoste>      not just the LP team
Feb 04 10:38:27 <flacoste>      otherwise, your branch isn't going to get reviewed
Feb 04 10:38:42 <flacoste>      that's how review allocation is done through Launchpad
Feb 04 10:38:46 <rockstar>      flacoste, well, it can be the LP team, just in case that person doesn't get to it in time (like in gmb's case).
Feb 04 10:38:55 <flacoste>      that doesn't work
Feb 04 10:39:02 <rockstar>      In that case, you ask someone else and they pick it up instead.
Feb 04 10:39:02 <mars>  flacoste, rockstar, would adding a "Pending review by" column to +activereviews help?
Feb 04 10:39:10 <flacoste>      and you assign it to them
Feb 04 10:39:16 <flacoste>      if it's assigned to the team
Feb 04 10:39:17 <barry> flacoste, 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 situation
Feb 04 10:39:19 <flacoste>      it's assigned to no one
Feb 04 10:39:21 <gary_poster>   barry: 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?
Feb 04 10:39:51 *       Disconnected (Connection reset by peer).
**** ENDING LOGGING AT Wed Feb  4 10:39:51 2009

**** BEGIN LOGGING AT Wed Feb  4 10:40:05 2009

Feb 04 10:40:05 *       Now talking on #launchpad-meeting
Feb 04 10:40:05 *       Topic for #launchpad-meeting is:  Launchpad Meeting Grounds | Channel logs: http://irclogs.ubuntu.com/ | Meeting Logs: http://www.novarata.net/mootbot/ | https://dev.launchpad.net/MeetingAgenda
Feb 04 10:40:05 *       Topic for #launchpad-meeting set by Rinchen at Tue Dec 16 14:28:48 2008
Feb 04 10:40:05 *       #launchpad-meeting :https://launchpad.net
Feb 04 10:40:12 <barry> dammit!
Feb 04 10:40:21 <barry> what did i miss? ;)
Feb 04 10:40:22 <rockstar>      It would be helpful if you could un-request someone to review it.
Feb 04 10:40:22 <flacoste>      barry: i only know of stub, but that's mainly because he's used to the old scheme
Feb 04 10:40:57 <barry> rockstar: that would be useful.  then stub could pick the next schedule ocr, which would work 90% of the time
Feb 04 10:41:04 <rockstar>      Because ideally, once every reviewer says "Approve" then the status of the BMP should be "approved"
Feb 04 10:41:33 <barry> flacoste: as an ocr, i'd be happy if there were a stub branch or two that were waiting for me when i started
Feb 04 10:41:38 <abentley>      gary_poster: Would it make sense for "claim review" to be separate from "perform review"?
Feb 04 10:41:47 <flacoste>      yes!
Feb 04 10:41:56 <flacoste>      there is no way to claim a review
Feb 04 10:41:57 <gary_poster>   abentley: I think that would be cool
Feb 04 10:41:59 <flacoste>      except by doing it
Feb 04 10:42:02 <barry> flacoste: i'd just pick them off first.  the question is, how best to seed the queue?
Feb 04 10:42:08 <flacoste>      or marking your vote as abstain
Feb 04 10:42:11 <flacoste>      which sounds weird
Feb 04 10:42:48 <barry> flacoste: 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 topic
Feb 04 10:43:06 <barry> flacoste: that sounds simple and i think it would work for me.  if that fails then we can try something else
Feb 04 10:43:24 <flacoste>      barry: i don't think making an exception for stub is useful at this point
Feb 04 10:43:37 <flacoste>      we just have to state that you have to find a reviewer for your branch
Feb 04 10:43:49 <flacoste>      so if there is no indivual reviewer assigned to it
Feb 04 10:44:01 <flacoste>      your branch is not going to be reviewed
Feb 04 10:44:05 <flacoste>      and run with it for a while
Feb 04 10:44:18 <barry> flacoste: stub was just an example.  i think most devs don't fall into dead zones
Feb 04 10:44:21 <flacoste>      it's possible that it's not a problem in practice for anybody to be able to find somebody to commit to a review
Feb 04 10:44:29 <bac>   reviewers will need to get in the habit of checking their +requestedreviews ... i know i've never looked at it before
Feb 04 10:44:34 <flacoste>      the problem here is that he was expecting somebody to pick it up, like it used to work
Feb 04 10:44:49 <rockstar>      bac, it's a relatively new view.
Feb 04 10:44:59 <flacoste>      the problem is that there was a subtle change in workflow without any formal announcement
Feb 04 10:45:18 <flacoste>      bac: well, if you commit to something, you commited to something
Feb 04 10:45:27 <barry> flacoste: i think i get what you're saying
Feb 04 10:45:37 <bac>   flacoste: well, i didn't commit to it if someone just assigns the review to me
Feb 04 10:45:38 <flacoste>      you only need to look at +requestedreviews if you have trouble remembering your commitments :-)
Feb 04 10:45:47 <flacoste>      bac: nobody would
Feb 04 10:45:58 <flacoste>      they would ask you permission to assign you to it
Feb 04 10:46:20 <bac>   that's not what barry suggested ^^, which seems reasonable to me
Feb 04 10:46:23 <barry> flacoste: 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 reviewer
Feb 04 10:46:26 <flacoste>      hey bac, can you review this branch for me (because you are OCR, or simply because you are cool)
Feb 04 10:46:29 <flacoste>      sure!
Feb 04 10:46:34 <flacoste>      then I assign the review to you
Feb 04 10:46:57 <bac>   flacoste: that's great if it is the flow we decide on.
Feb 04 10:47:02 <barry> flacoste: also would work: an email saying "hey barry, you're on call tomorrow so i left you a present"
Feb 04 10:47:19 <flacoste>      well, it's the one that we seem to be using and that is possible given the current tool
Feb 04 10:47:30 <flacoste>      (since we don't want to manage a queue somewhere else)
Feb 04 10:47:39 <bac>   ah, nm.  being assigned a review will generate email addressed directly to me, so i'd know about it.
Feb 04 10:48:02 <barry> bac: yeah
Feb 04 10:48:08 *       bac still like the new +requestedreviews view...
Feb 04 10:48:55 <rockstar>      bac, yeah, I do too, but I think it's important to have that acknowledgement/handshake in the review process.
Feb 04 10:49:05 <barry> flacoste: 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 first
Feb 04 10:49:20 <BjornT>        the 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 time
Feb 04 10:49:24 <bac>   Q:  i see ~launchpad/+requestedreviews has very few listed, which means ~launchpad is not automatically being added to some new MPs.  why is that?
Feb 04 10:49:53 <bac>   BjornT: only if you're not around for the OCR.
Feb 04 10:50:16 <abentley>      bac: Most likely because reviews created by email don't have that set.
Feb 04 10:50:37 <BjornT>        bac: or the branch is around too late, so that you have to wait the next OCRs to start their shift
Feb 04 10:50:40 <flacoste>      barry: i'll start a thread
Feb 04 10:50:44 <barry> bac: there are also some cases where mp's won't auto-assign a team
Feb 04 10:50:45 <bac>   abentley: i created one this morning using lpsend
Feb 04 10:51:05 <barry> flacoste: thanks
Feb 04 10:51:08 <flacoste>      BjornT: that's what I usually do, if i'm too late, i'll just ask somebody the next day
Feb 04 10:51:14 <abentley>      bac: and it automatically requested ~launchpad to review?
Feb 04 10:51:17 <barry> [ACTION] flacoste to take dead zone reviews discussion to ml
Feb 04 10:51:17 <MootBot>       ACTION received:  flacoste to take dead zone reviews discussion to ml
Feb 04 10:51:30 <bac>   abentley: it did not.  i expected it would.
Feb 04 10:51:43 <bac>   abentley: wrong assumption?
Feb 04 10:51:48 <BjornT>        flacoste: right. it would still be nicer to ask a group of people, instead of a single person.
Feb 04 10:51:51 <abentley>      bac: Wrong assumption.
Feb 04 10:52:01 <abentley>      bac: Not that it shouldn't, but it doesn't.
Feb 04 10:52:09 <bac>   abentley: ok.
Feb 04 10:52:17 <flacoste>      BjornT: yeah, but my experience is that this doesn't really work in practice
Feb 04 10:52:33 *       andrea-bs (n=andrea-b@ubuntu/member/beeseek.developer.andrea-bs) has joined #launchpad-meeting
Feb 04 10:52:39 <flacoste>      at least, that's my experience with the advent of OCR
Feb 04 10:52:52 <flacoste>      sure, the OCR might swap it between themselve
Feb 04 10:52:58 <flacoste>      but only because the first one agreed to it
Feb 04 10:52:59 <BjornT>        flacoste: that's why i said "would" :) it doesn't work atm, but it would be nice if it would
Feb 04 10:53:07 <flacoste>      i agree
Feb 04 10:53:22 <barry> okay, 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-code
Feb 04 10:53:30 <barry> #endmeeting

ReviewerMeeting20090204 (last edited 2009-02-10 21:54:41 by barry)