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