/srv/irclogs.ubuntu.com/2010/11/17/#launchpad-reviews.txt

=== 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
bacjtv: r=bac06:24
jtvthanks bac!06:27
=== 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
stubjtv: https://code.launchpad.net/~stub/launchpad/bug-675562-readonly-violation/+merge/4103409:27
jtvstub: call first, then I'll review.09:31
jtvstub: 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:36
stubIt 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:37
jtvstub: I'll leave it to you—depends on how well-established the rule is that master stores don't implement ISlaveStore.09:43
=== 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
jtvstub: nice branch btw.09:44
jtvOne other thing: in lib/canonical/launchpad/webapp/adapter.py, the "if/elif" should have an "else" as per our coding standard.09:45
stubHmm... 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
stubAlthough it doesn't work that way at the moment.09:45
jtvstub: ironically, my review got a warning that we're going into read-only mode.  But made it just in time.09:48
=== 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
henningebigjools: 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
henningejust as sanity check question10:18
bigjoolshenninge: no, I want that whole method eliminated10:21
henningeok10:21
bigjoolshenninge: because it fails the job itself too10:21
henningeI see10:21
=== danilo_ is now known as danilos
jtvhi danilos11:02
daniloshi jtv11:03
=== 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
jtvEdwinGrubbs: are you not reviewing today?13:24
jtvOr jelmer?13:24
jelmerjtv: Yes, I am. I figured you were still around and it was quiet.13:25
jelmerjtv: I can have a look at your MP.13:25
jtvjelmer: I am, but I can't review my own.  :-)  Thanks.13:25
=== 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
EdwinGrubbsjtv: my work day hasn't quite started yet.14:08
=== salgado-physio is now known as salgado
jcsackettjelmer, (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/4101114:18
=== 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
jelmerjcsackett: thanks14:19
jcsackettjelmer: shouldn't i be thanking you? :-P14:19
=== mrevell-lunch is now known as mrevell
jelmerjcsackett: 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
jcsackettjelmer: aaaah. :-)14:21
benjiEdwinGrubbs: when do you want me to start doing mentored reviews?  I assume it'll be on days that you are OCR.14:44
EdwinGrubbsbenji: oh, that's a good idea. I'll put you in the oncall list with me.14:44
=== 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
benjiEdwinGrubbs: 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:46
EdwinGrubbsbenji: well, the reviewers meeting is coming up in 13 minutes in #launchpad-meeting.14:47
benjiah, good call14:47
=== matsubara is now known as matsubara-lunch
EdwinGrubbsbenji: 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:00
=== matsubara-lunch is now known as matsubara
jtvwow, 3 ocrs15:57
jelmerwell, one is *16:02
EdwinGrubbsbenji: can you review https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/4101116:40
benjiEdwinGrubbs: sure!16:40
EdwinGrubbsbenji: make sure to move jcsacket from the queue to reviewing in the channel topic, and click "Claim review" on the mp16:41
benjik16:41
=== 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
bigjoolshello, can I have a review of this please: https://code.launchpad.net/~julian-edwards/launchpad/disable-failures-bm-bug-676479/+merge/4106817:09
jelmerbigjools: I'll do a review before I EOD17:16
benjiEdwinGrubbs: I've done my review: https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/41011.17:17
bigjoolsthanks jelmer17:18
=== benji is now known as benji-lunch
EdwinGrubbsbenji-lunch: when you review a branch as a mentee, you should set the review type to "code*" instead of "code".18:16
=== benji-lunch is now known as benji
benjiEdwinGrubbs: will do18:27
EdwinGrubbsbenji: 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
=== EdwinGrubbs is now known as Edwin-lunch
benjicool18:32
=== 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]
jcsackettEdwinGrubbs: I fixed the bad query you pointed out.19:39
jcsackettchanges pushed.19:39
=== 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
sinzuibenji, Edwin-lunch I placed my enable-tests-0 branch into the queue19:42
=== Edwin-lunch is now known as EdwinGrubbs
benjisinzui: cool, I'll take a look at it momentarily.19:43
* benji considers writing an irssi plugin for review topic handling.19:43
=== 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
jcsackettbenji: if you write that, pass it around. :-)19:45
benji:)  will do19:45
=== deryck[lunch] is now known as deryck
=== salgado is now known as salgado-afk
benjisinzui: I only had one question on https://code.launchpad.net/~sinzui/launchpad/enable-tests-0/+merge/41088.20:40
benjiEdwinGrubbs: I'v reviewed sinzui's branch at https://code.launchpad.net/~sinzui/launchpad/enable-tests-0/+merge/4108820:41
* EdwinGrubbs looks20:41
gary_posterbenji, may I add https://code.launchpad.net/~gary/launchpad/bug676489/+merge/41069 to your queue?  Hopefully quick one, only 75 line diff20:49
benjigary_poster: sure20:49
gary_postercool20:49
=== 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
benjijcsackett: I commented on https://code.edge.launchpad.net/~jcsackett/launchpad/convert-sql-627631-storm/+merge/4101120:54
=== 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
* benji requires another Dr. Pepper20:59
jcsackettbenji: 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:-P21:00
=== Ursinha is now known as Ursinha-bbk
=== Ursinha-bbk is now known as Ursinha-bbl
=== matsubara is now known as matsubara-afk
benjigary_poster: your branch looks good; I did have one small thought which I included in a comment.21:10
jtvbenji: 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/4110822:06
benjijtv: 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
jtvbenji: then between the two of us we may just be halving build speed today.22:07
jtvSorry, build _time_ :)22:07
benji:)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
* benji has flashbacks from his days of being knee deep in Makefiles.22:11
jtvFond memories, eh?22:12
benjiyou 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
jtvchuckle22:14
benjijtv: I commented (approvingly) on your branch: https://code.launchpad.net/~jtv/launchpad/parallel-make/+merge/4110822:44
jtvthanks benji!22:44
benjiEdwinGrubbs: it should be ready for you to review now.22:44
=== 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
EdwinGrubbsI'll look at it22:45
benjithanks22:45
gary_postermany thanks, benji and Edwin-afk23:08
gary_posterEdwinGrubbs:23:08
gary_posterI meant thank you :-P23:09
EdwinGrubbsI really wish I could get freenode to kill Edwin-afk. It has been there for days.23:10
=== 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

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!