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