=== jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: bryce || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === bac changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: bryce, jtv(bac) || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === bac changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: bryce || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [06:24] jtv: r=bac [06:27] thanks bac! === jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === jtv is now known as jtv-eat [09:27] jtv: https://code.launchpad.net/~stub/launchpad/bug-675562-readonly-violation/+merge/41034 [09:31] stub: call first, then I'll review. [09:36] 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] 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] stub: I'll leave it to you—depends on how well-established the rule is that master stores don't implement ISlaveStore. === jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: stub || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === Guest8056 is now known as jelmer [09:44] stub: nice branch btw. [09:45] One other thing: in lib/canonical/launchpad/webapp/adapter.py, the "if/elif" should have an "else" as per our coding standard. [09:45] 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] Although it doesn't work that way at the moment. [09:48] stub: ironically, my review got a warning that we're going into read-only mode. But made it just in time. === jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [10:18] 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] just as sanity check question [10:21] henninge: no, I want that whole method eliminated [10:21] ok [10:21] henninge: because it fails the job itself too [10:21] I see === danilo_ is now known as danilos [11:02] hi danilos [11:03] hi jtv === matsubara-afk is now known as matsubara === jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === salgado is now known as salgado-physio === jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [13:24] EdwinGrubbs: are you not reviewing today? [13:24] Or jelmer? [13:25] jtv: Yes, I am. I figured you were still around and it was quiet. [13:25] jtv: I can have a look at your MP. [13:25] jelmer: I am, but I can't review my own. :-) Thanks. === jelmer changed the topic of #launchpad-reviews to: On call: jelmer || Reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mrevell is now known as mrevell-lunch === Ursinha-dinner is now known as Ursinha [14:08] jtv: my work day hasn't quite started yet. === salgado-physio is now known as salgado [14:18] 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 === jcsackett changed the topic of #launchpad-reviews to: On call: jelmer || Reviewing: jtv || queue: [jcsackett] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:19] jcsackett: thanks [14:19] jelmer: shouldn't i be thanking you? :-P === mrevell-lunch is now known as mrevell [14:21] 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] jelmer: aaaah. :-) [14:44] EdwinGrubbs: when do you want me to start doing mentored reviews? I assume it'll be on days that you are OCR. [14:44] benji: oh, that's a good idea. I'll put you in the oncall list with me. === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jelmer, benji, Edwin || Reviewing: jtv || queue: [jcsackett] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:46] 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] benji: well, the reviewers meeting is coming up in 13 minutes in #launchpad-meeting. [14:47] ah, good call === matsubara is now known as matsubara-lunch [15:00] 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. === matsubara-lunch is now known as matsubara [15:57] wow, 3 ocrs [16:02] well, one is * [16:40] benji: can you review https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011 [16:40] EdwinGrubbs: sure! [16:41] benji: make sure to move jcsacket from the queue to reviewing in the channel topic, and click "Claim review" on the mp [16:41] k === benji changed the topic of #launchpad-reviews to: On call: jelmer, benji, Edwin || Reviewing: jtv, jcsackett || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [17:09] 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] bigjools: I'll do a review before I EOD [17:17] EdwinGrubbs: I've done my review: https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011. [17:18] thanks jelmer === benji is now known as benji-lunch [18:16] benji-lunch: when you review a branch as a mentee, you should set the review type to "code*" instead of "code". === benji-lunch is now known as benji [18:27] EdwinGrubbs: will do [18:32] 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. === EdwinGrubbs is now known as Edwin-lunch [18:32] cool === jelmer changed the topic of #launchpad-reviews to: On call: benji, Edwin || Reviewing: jcsackett || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === deryck is now known as deryck[lunch] [19:39] EdwinGrubbs: I fixed the bad query you pointed out. [19:39] changes pushed. === sinzui changed the topic of #launchpad-reviews to: On call: benji, Edwin || Reviewing: jcsackett || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [19:42] benji, Edwin-lunch I placed my enable-tests-0 branch into the queue === Edwin-lunch is now known as EdwinGrubbs [19:43] sinzui: cool, I'll take a look at it momentarily. [19:43] * benji considers writing an irssi plugin for review topic handling. === benji changed the topic of #launchpad-reviews to: On call: benji, Edwin || Reviewing: jcsackett, sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [19:45] benji: if you write that, pass it around. :-) [19:45] :) will do === deryck[lunch] is now known as deryck === salgado is now known as salgado-afk [20:40] sinzui: I only had one question on https://code.launchpad.net/~sinzui/launchpad/enable-tests-0/+merge/41088. [20:41] 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] 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] gary_poster: sure [20:49] cool === gary_poster changed the topic of #launchpad-reviews to: On call: benji, Edwin || Reviewing: jcsackett, sinzui || queue: [gary] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:54] jcsackett: I commented on https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011 === benji changed the topic of #launchpad-reviews to: On call: benji, Edwin || Reviewing: jcsackett, sinzui, gary || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:59] * benji requires another Dr. Pepper [21:00] 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] :-P === Ursinha is now known as Ursinha-bbk === Ursinha-bbk is now known as Ursinha-bbl === matsubara is now known as matsubara-afk [21:10] gary_poster: your branch looks good; I did have one small thought which I included in a comment. [22:06] 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] 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] \o/ [22:07] benji: then between the two of us we may just be halving build speed today. [22:07] Sorry, build _time_ :) [22:07] :) === benji changed the topic of #launchpad-reviews to: On call: benji, Edwin || Reviewing: gary, jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [22:11] * benji has flashbacks from his days of being knee deep in Makefiles. [22:12] Fond memories, eh? [22:14] 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] chuckle [22:44] jtv: I commented (approvingly) on your branch: https://code.launchpad.net/~jtv/launchpad/parallel-make/+merge/41108 [22:44] thanks benji! [22:44] EdwinGrubbs: it should be ready for you to review now. === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: benji, Edwin || Reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [22:45] I'll look at it [22:45] thanks [23:08] many thanks, benji and Edwin-afk [23:08] EdwinGrubbs: [23:09] I meant thank you :-P [23:10] I really wish I could get freenode to kill Edwin-afk. It has been there for days. === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews