= ReviewerMeeting20090121 =

== summary ==

 * gmb, al-maisan move ocr to tuesday for two weeks
 * abentley points out `status approved` command and discusses the difference
 between that and `review approve`.

== logs ==

{{{
Jan 21 10:00:27 <barry>	#startmeeting
Jan 21 10:01:00 <barry>	mootbot, like gwb is an ex-mootbot
Jan 21 10:01:08 <bigjools>	the mystery of the missing mootbot
Jan 21 10:01:13 <rockstar>	mootbot is dead.
Jan 21 10:01:17 <rockstar>	I killed him.
Jan 21 10:01:19 <barry>	anyway.  welcome to this week's ameu reviewer's meeting.  who's here today?
Jan 21 10:01:23 <sinzui>	me
Jan 21 10:01:25 <bigjools>	me
Jan 21 10:01:26 <rockstar>	me
Jan 21 10:01:26 <mars>	me
Jan 21 10:01:28 <gary_poster>	me
Jan 21 10:01:28 <al-maisan>	miau
Jan 21 10:01:28 <adeuring>	me
Jan 21 10:01:28 <abentley>	me
Jan 21 10:01:29 <allenap>	me
Jan 21 10:01:34 <salgado>	me
Jan 21 10:02:17 <bac>	me
Jan 21 10:02:52 <intellectronica>	me
Jan 21 10:03:05 <barry>	BjornT, cprov, danilos ping
Jan 21 10:03:15 <gmb>	me
Jan 21 10:03:15 *	flacoste (n=francis@canonical/launchpad/flacoste) has joined #launchpad-meeting
Jan 21 10:03:28 <danilos>	me
Jan 21 10:03:28 <flacoste>	me
Jan 21 10:03:31 <barry>	EdwinGrubbs: ping
Jan 21 10:03:35 <danilos>	(though I am likely to be on and off)
Jan 21 10:03:43 <EdwinGrubbs>	me
Jan 21 10:03:46 <barry>	np, today's a light agenda
Jan 21 10:03:52 <barry>	[TOPIC] agenda
Jan 21 10:04:01 <barry>	 * Roll call
Jan 21 10:04:01 <barry>	 * mars to stop ocr, will review js on call until more reviewers are trained
Jan 21 10:04:01 <barry>	 * Peanut gallery (anything not on the agenda)
Jan 21 10:04:01 <barry>	 * Action items
Jan 21 10:04:01 <barry>	 * Mentoring update
Jan 21 10:04:13 <barry>	oops.  ignore the mars item, we did that last week
Jan 21 10:04:22 <mars>	*whew*
Jan 21 10:04:31 <barry>	let's skip around a bit...  i suck today
Jan 21 10:04:37 <barry>	[TOPIC] action items
Jan 21 10:04:43 <barry>	 * barry to look into techniques for eliminating back-patching of schema types (avoiding circular imports)
Jan 21 10:04:59 <barry>	i actually started to look at this and i might have a branch for review later this week
Jan 21 10:05:08 <bigjools>	\o/
Jan 21 10:05:17 <barry>	 * barry to add `pretty()` functions to reviewers docs
Jan 21 10:05:19 <barry>	not done
Jan 21 10:05:25 <barry>	 * flacoste to work on API reviewer cheat sheet
Jan 21 10:05:47 <flacoste>	*sigh*
Jan 21 10:06:18 <barry>	no worries.  do you want to keep it on the list?
Jan 21 10:06:31 <flacoste>	yeah, for two more weeks
Jan 21 10:06:51 <barry>	you got it :)
Jan 21 10:07:00 <barry>	[TOPIC] mentoring update
Jan 21 10:07:11 <barry>	any word from mentors or mentats?
Jan 21 10:07:44 <gmb>	al-maisan's doing very well now that the volume of reviews is back up to something approaching normal
Jan 21 10:08:01 <gmb>	Mondays aren't the ideal day for mentoring, I think; they can be quite quiet.
Jan 21 10:08:23 *	abentley agrees they are quiet.
Jan 21 10:08:44 <barry>	should we move you guys to get better coverage?
Jan 21 10:09:28 <barry>	i think adeuring was looking for a monday slot
Jan 21 10:09:54 <gmb>	barry: I'm not sure, it depends on how al-maisan feels.
Jan 21 10:09:57 <barry>	we also have euro/tues free
Jan 21 10:10:00 <adeuring>	adeuring: well, I wouldn't oopose to taking Monday
Jan 21 10:10:20 <adeuring>	...or Tuesday or Wednesday
Jan 21 10:10:29 <al-maisan>	gmb: I am your apprentice, so will tag along :)
Jan 21 10:10:33 <bigjools>	any day ending in a "y"
Jan 21 10:10:44 <gmb>	:)
Jan 21 10:11:01 <gmb>	al-maisan, barry Maybe it's worth moving to Tuesday for a week or two.
Jan 21 10:11:01 <barry>	bigjools: thank goodness that leaves satursun out
Jan 21 10:11:39 <barry>	gmb, al-maisan ok. let's move you guys to tuesday for two weeks to get more mentoring in
Jan 21 10:11:57 <gmb>	Cool.
Jan 21 10:12:02 <barry>	adeuring: let's keep you at friday for now and then depending on how it goes with gmb and al-maisan we could switch you after that
Jan 21 10:12:02 <al-maisan>	fine :)
Jan 21 10:12:05 <gmb>	I'll update the OCR page.
Jan 21 10:12:09 <adeuring>	barry: OK
Jan 21 10:12:09 <barry>	gmb: awesome, thanks
Jan 21 10:12:28 <barry>	any other mentoring issues?
Jan 21 10:13:37 <barry>	guess not!
Jan 21 10:13:45 <barry>	[TOPIC] peanut gallery
Jan 21 10:13:52 <abentley>	I have a PSA
Jan 21 10:13:59 <barry>	well, that's all i have on my agenda, i open the floor to y'all
Jan 21 10:14:08 <barry>	abentley: go ahead
Jan 21 10:14:13 *	al-maisan wonders what a PSA is..?
Jan 21 10:14:24 <gmb>	al-maisan: public service announcement
Jan 21 10:14:37 <al-maisan>	ah!
Jan 21 10:14:37 <abentley>	When you approve a merge proposal, please mark its status approved as well.
Jan 21 10:14:51 <abentley>	If you are using email, you can use the "status approved" command.
Jan 21 10:15:04 <rockstar>	Now you know, and knowing is half the battle! Gee Eye JOOOOOOOOOOE
Jan 21 10:15:07 <abentley>	This will remove the proposal from the list of active reviews.
Jan 21 10:15:08 <barry>	abentley: that is a continuing source of pain ;)
Jan 21 10:15:29 <abentley>	Which makes it easier to see what needs to be reviewed.
Jan 21 10:15:33 <bac>	abentley: +1
Jan 21 10:15:57 <gmb>	Nurse, nurse! rockstar's out of bed again...
Jan 21 10:16:28 <rockstar>	gmb: did you not watch GI Joe, with the PSAs at the end?
Jan 21 10:16:29 <abentley>	You can use "review approve" and "status approved" in the same email.
Jan 21 10:16:45 <gmb>	rockstar: Call it cultural differences :)
Jan 21 10:16:46 <abentley>	Just on different lines.
Jan 21 10:16:49 <salgado>	abentley, why do we need both?
Jan 21 10:17:11 <rockstar>	I know thumper talked about doing it automatically, but we have some details to figure out first.
Jan 21 10:17:27 <al-maisan>	abentley: do you need both?
Jan 21 10:17:37 <abentley>	salgado: Because one is a reviewer's opinion, and one is the status of the merge proposal.
Jan 21 10:17:41 <rockstar>	salgado: one is the status of the CodeReviewVote, the other is for the status of the BranchMergeProposal
Jan 21 10:17:42 <sinzui>	salgado: some project may require two more more reviews to be approved before the status is really approved.
Jan 21 10:17:43 <EdwinGrubbs>	abentley: btw, doesn't there need to be a leading space in the email commands, so it should be " review approve"
Jan 21 10:17:47 <flacoste>	i think you only need status approve
Jan 21 10:17:55 <salgado>	I see
Jan 21 10:17:55 <abentley>	EdwinGrubbs: Yes.
Jan 21 10:18:00 <flacoste>	iirc, it also automatically approve the review
Jan 21 10:18:13 <rockstar>	flacoste: not true currently.
Jan 21 10:18:18 <abentley>	salgado: There are two reviews in many cases.
Jan 21 10:18:23 <salgado>	I've been using only "status approve"
Jan 21 10:18:57 <barry>	salgado: i guess that approves the mp without setting your review status to approve...?
Jan 21 10:19:04 <rockstar>	salgado: it'd be " status approved" - note the tense
Jan 21 10:19:06 <abentley>	salgado: Other projects may have different rules about how many reviews are required, whether reviewers can veto, etc.
Jan 21 10:19:32 <bigjools>	maybe have a per-project policy that can be set then
Jan 21 10:19:39 <abentley>	barry: Right.  It's like: "I don't approve of this, but merge it anyway."
Jan 21 10:20:07 <flacoste>	sure, i thought thumper said it did both
Jan 21 10:20:12 <flacoste>	?
Jan 21 10:20:27 <salgado>	abentley, I don't see it that way.  I see it more as an indication that the reviewer didn't know there were two separate things to approve
Jan 21 10:20:30 <barry>	abentley: and eventually we'll be able to specify those workflows and have it all work automatically, right? <wink>
Jan 21 10:20:41 <rockstar>	flacoste: it does in cases where you voted needs_fixing, and then revoted approve
Jan 21 10:20:54 <abentley>	barry: That's a good question.  The mandate to avoid imposing policy was from on high.
Jan 21 10:21:15 <barry>	abentley: not imposing policy, but providing the mechanisms for projects to specify their policy
Jan 21 10:21:18 <rockstar>	barry: eventually, given enough time, Launchpad will support direct teleportation to sprints.
Jan 21 10:21:24 <barry>	abentley: but i think that's also frowned on :/
Jan 21 10:21:43 <abentley>	barry: You may not from my work on Bundle Buggy that I think it makes a lot of sense to have policy about what is needed to approve a merge proposal.
Jan 21 10:21:46 <barry>	rockstar: thank goodness, 'cause i'm running out of my little round friends
Jan 21 10:22:29 <barry>	abentley: in this case, it could be as simple as a count of the number of approved reviews.  i don't even care about the rejected ones
Jan 21 10:22:44 <al-maisan>	the email generated by the webapp says: "Review: Approve" BTW .. that means we cannot use that any more?
Jan 21 10:23:09 <rockstar>	barry: in Entertainer's case, a rejected or a needs_fixing prevents the branch from landing regardless of the approveds...
Jan 21 10:23:45 <abentley>	barry: So in LP, we have mentor / mentat reviews, which are one special case.  And we have database reviews, which are another.
Jan 21 10:24:16 *	barry invokes the 80/20 rule
Jan 21 10:24:22 <rockstar>	al-maisan: that is just the output email.  The input in " review approve"
Jan 21 10:25:08 <salgado>	does "vote approve" work as well?
Jan 21 10:25:13 <abentley>	rockstar is working on exposing BMPs through the API.  Presumably he could write a script to enforce a policy.
Jan 21 10:25:28 <barry>	abentley, rockstar +1 !
Jan 21 10:25:33 <rockstar>	abentley: yes, that's an idea.
Jan 21 10:25:48 <rockstar>	salgado: vote is deprecated.  Use review.
Jan 21 10:26:00 <salgado>	will do
Jan 21 10:26:37 <barry>	abentley: cool, thanks for that psa
Jan 21 10:26:44 <abentley>	barry: np
Jan 21 10:26:47 <barry>	anything else on this or other topics?
Jan 21 10:27:22 <abentley>	barry: I've just started work on generating diffs for all merge proposals.
Jan 21 10:27:29 *	flacoste cheers
Jan 21 10:27:33 <barry>	abentley: yay!
Jan 21 10:27:44 <flacoste>	i propose a virtual wave for abentley!
Jan 21 10:27:56 <al-maisan>	:)
Jan 21 10:28:21 <barry>	everyone send abentley an e-beer
Jan 21 10:29:17 <barry>	are we done?
Jan 21 10:29:19 <barry>	5
Jan 21 10:29:28 <barry>	4
Jan 21 10:29:39 <barry>	3
Jan 21 10:29:48 <barry>	2
Jan 21 10:29:55 <barry>	1
Jan 21 10:29:57 <barry>	#endmeeting
}}}