[06:24] <bac> jtv: r=bac
[06:27] <jtv> thanks bac!
[09:27] <stub> jtv: https://code.launchpad.net/~stub/launchpad/bug-675562-readonly-violation/+merge/41034
[09:31] <jtv> stub: call first, then I'll review.
[09:36] <jtv> stub: actually, looking already…  would it make sense in the doctest to verify that the store you get in read-only mode does not implement IMasterStore (in addition to the current test that it does implement ISlaveStore)?
[09:37] <stub> It would be redundant since a store isn't that schizophrenic . I can change the test if you think checking for not IMasterStore is clearer.
[09:43] <jtv> stub: I'll leave it to you—depends on how well-established the rule is that master stores don't implement ISlaveStore.
[09:44] <jtv> stub: nice branch btw.
[09:45] <jtv> One other thing: in lib/canonical/launchpad/webapp/adapter.py, the "if/elif" should have an "else" as per our coding standard.
[09:45] <stub> Hmm... I guess technically ISlaveStore(Foo) could return a master store, and if so, would be violating contract if the result didn't implement the interface.
[09:45] <stub> Although it doesn't work that way at the moment.
[09:48] <jtv> stub: ironically, my review got a warning that we're going into read-only mode.  But made it just in time.
[10:18] <henninge> bigjools: without know too much about the builder but shouldn't the cowboy be in assessFailuresCount where it disables the builder instead of not assessing the failure countes at all?
[10:18] <henninge> just as sanity check question
[10:21] <bigjools> henninge: no, I want that whole method eliminated
[10:21] <henninge> ok
[10:21] <bigjools> henninge: because it fails the job itself too
[10:21] <henninge> I see
[11:02] <jtv> hi danilos
[11:03] <danilos> hi jtv
[13:24] <jtv> EdwinGrubbs: are you not reviewing today?
[13:24] <jtv> Or jelmer?
[13:25] <jelmer> jtv: Yes, I am. I figured you were still around and it was quiet.
[13:25] <jelmer> jtv: I can have a look at your MP.
[13:25] <jtv> jelmer: I am, but I can't review my own.  :-)  Thanks.
[14:08] <EdwinGrubbs> jtv: my work day hasn't quite started yet.
[14:18] <jcsackett> jelmer, (or EdwinGrubbs when your day starts): i have an MP to throw on the queue. https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011
[14:19] <jelmer> jcsackett: thanks
[14:19] <jcsackett> jelmer: shouldn't i be thanking you? :-P
[14:21] <jelmer> jcsackett: I guess it's a bit premature, if I haven't looked at your MP yet, but I was thanking you for improving lp :-)
[14:21] <jcsackett> jelmer: aaaah. :-)
[14:44] <benji> EdwinGrubbs: when do you want me to start doing mentored reviews?  I assume it'll be on days that you are OCR.
[14:44] <EdwinGrubbs> benji: oh, that's a good idea. I'll put you in the oncall list with me.
[14:46] <benji> EdwinGrubbs: I've read over the various reviewer docs and kept an eye on reviews as they've gone through my inbox.  Is there anything else you'd like me to read/do to prepare?
[14:47] <EdwinGrubbs> benji: well, the reviewers meeting is coming up in 13 minutes in #launchpad-meeting.
[14:47] <benji> ah, good call
[15:00] <EdwinGrubbs> benji: one of school of though on doing oncall reviews is to just ask a bunch of questions to verify that the coder's logic is sound as opposed to analyzing the patch in depth for the logic, i.e. there is a precedent for reviewers pestering the coder with questions that aren't though through. This should make it less tiresome to do reviews on parts of the code you are less familiar with.
[15:57] <jtv> wow, 3 ocrs
[16:02] <jelmer> well, one is *
[16:40] <EdwinGrubbs> benji: can you review https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011
[16:40] <benji> EdwinGrubbs: sure!
[16:41] <EdwinGrubbs> benji: make sure to move jcsacket from the queue to reviewing in the channel topic, and click "Claim review" on the mp
[16:41] <benji> k
[17:09] <bigjools> hello, can I have a review of this please: https://code.launchpad.net/~julian-edwards/launchpad/disable-failures-bm-bug-676479/+merge/41068
[17:16] <jelmer> bigjools: I'll do a review before I EOD
[17:17] <benji> EdwinGrubbs: I've done my review: https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011.
[17:18] <bigjools> thanks jelmer
[18:16] <EdwinGrubbs> benji-lunch: when you review a branch as a mentee, you should set the review type to "code*" instead of "code".
[18:27] <benji> EdwinGrubbs: will do
[18:32] <EdwinGrubbs> benji: I'm going to lunch now, but I added a comment in the review about a problem that I really wish storm would warn us about. We can discuss it more when I get back.
[18:32] <benji> cool
[19:39] <jcsackett> EdwinGrubbs: I fixed the bad query you pointed out.
[19:39] <jcsackett> changes pushed.
[19:42] <sinzui> benji, Edwin-lunch I placed my enable-tests-0 branch into the queue
[19:43] <benji> sinzui: cool, I'll take a look at it momentarily.
[19:43]  * benji considers writing an irssi plugin for review topic handling.
[19:45] <jcsackett> benji: if you write that, pass it around. :-)
[19:45] <benji> :)  will do
[20:40] <benji> sinzui: I only had one question on https://code.launchpad.net/~sinzui/launchpad/enable-tests-0/+merge/41088.
[20:41] <benji> EdwinGrubbs: I'v reviewed sinzui's branch at https://code.launchpad.net/~sinzui/launchpad/enable-tests-0/+merge/41088
[20:41]  * EdwinGrubbs looks
[20:49] <gary_poster> benji, may I add https://code.launchpad.net/~gary/launchpad/bug676489/+merge/41069 to your queue?  Hopefully quick one, only 75 line diff
[20:49] <benji> gary_poster: sure
[20:49] <gary_poster> cool
[20:54] <benji> jcsackett: I commented on https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011
[20:59]  * benji requires another Dr. Pepper
[21:00] <jcsackett> benji: i saw. actually it suggests i got my ec2 results mixed up. i have another branch that's *very* similarly named that was being developed more or less at the same time as this one.
[21:00] <jcsackett> :-P
[21:10] <benji> gary_poster: your branch looks good; I did have one small thought which I included in a comment.
[22:06] <jtv> benji: I couldn't resist one more try… this one saves me over a minute of build time.  Wanna review?  https://code.launchpad.net/~jtv/launchpad/parallel-make/+merge/41108
[22:07] <benji> jtv: sure (by the way, I've figured out why so much option parsing is taking place and I think I can avoid it all)
[22:07] <jtv> \o/
[22:07] <jtv> benji: then between the two of us we may just be halving build speed today.
[22:07] <jtv> Sorry, build _time_ :)
[22:07] <benji> :)
[22:11]  * benji has flashbacks from his days of being knee deep in Makefiles.
[22:12] <jtv> Fond memories, eh?
[22:14] <benji> you could say that... I was an absolute expert in make for about six weeks; most of those brain cells have comitted suicide since then.
[22:14] <jtv> chuckle
[22:44] <benji> jtv: I commented (approvingly) on your branch: https://code.launchpad.net/~jtv/launchpad/parallel-make/+merge/41108
[22:44] <jtv> thanks benji!
[22:44] <benji> EdwinGrubbs: it should be ready for you to review now.
[22:45] <EdwinGrubbs> I'll look at it
[22:45] <benji> thanks
[23:08] <gary_poster> many thanks, benji and Edwin-afk
[23:08] <gary_poster> EdwinGrubbs:
[23:09] <gary_poster> I meant thank you :-P
[23:10] <EdwinGrubbs> I really wish I could get freenode to kill Edwin-afk. It has been there for days.