[04:13] StevenK: want another code review to do? [04:13] wallyworld: No? :-) [04:13] StevenK: that's ok. i thought i'd offer it to you first [04:13] Actually, let me ask a different question. "Do I have a choice?" :-) [04:14] wallyworld: But yes, I'll take a look [04:14] StevenK: of course there's a choice :-) . want wasn't sure if you were still needed to do them as part of the metoring [04:14] StevenK: if you want :-) https://code.edge.launchpad.net/~wallyworld/launchpad/recipe-find-related-branches/+merge/42097 [04:15] StevenK: feel free to pass if you want. i won't be offended :-) [04:17] wallyworld: You mention a bug in the summary, but you don't link it to the branch? :-) [04:20] wallyworld: I'm only surprised you have to kick the security proxy out of the way so much [04:27] StevenK: linked now [04:28] StevenK: what i found was that without removing the proxy, stuff would break when running the tests. can't recall the exact error now [04:58] wallyworld: Perhaps this means the zcml should be tweaked instead of telling zope to get out of your way? [04:59] StevenK: yes, perhaps. i can take another look [04:59] wallyworld: Anyway, +1'd, but that would be cool [05:00] StevenK: i guess, given in some places we need to removeSecurityProxy, i'm unsure what the guiding principals are as to when to go naked vs tweak zcml [05:01] thanks for the review btw === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [11:45] adeuring: Can I throw a review your way? [11:46] wgrant: sure; I'm currently reviewing an MP from EdwinGrubbs, but i'm nearly finished === wgrant changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: - || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [11:47] https://code.launchpad.net/~wgrant/launchpad/bug-675621-packages-binary-scaling/+merge/42607 is the MP. === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [12:03] * adeuring needs to remember that C-Y does not paste stuff from the clipboard in most programs [12:22] wgrant: r=me === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [12:23] adeuring: Thanks! [12:23] adeuring: Could you please throw it at EC2? [12:23] wgrant: sure [12:42] adeuring: Howdy. Can I have a review for a fairly trivial JS branch? https://code.edge.launchpad.net/~gmb/launchpad/lazr-wizard-linkage/+merge/42616 === mrevell is now known as mrevell-lunch [12:58] gmb: sure [12:59] Thanks [13:15] gmb: r=me [13:17] adeuring: Thanks. === bac changed the topic of #launchpad-reviews to: On call: adeuring,bac || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [13:32] I was trying to get Stuart to review this, since he told me where to stick my new code, but it seems he's already gone: https://code.launchpad.net/~jtv/launchpad/bug-684669/+merge/42621 === vednis is now known as mars === mrevell-lunch is now known as mrevell [13:53] hi adeuring. much happening today? [13:53] bac: well, three reviews so far [13:53] adeuring: hopping! [13:57] jtv: i'll look at it [13:57] thanks bac === bac changed the topic of #launchpad-reviews to: On call: adeuring,bac || Reviewing: -,jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-lunch [14:33] jtv: are there any tests where you could demonstrate this loggin? [14:34] bac: I could probably arrange for one. [14:34] Wasn't sure it was needed, since this will be exercised all the time anyway. [14:35] have you shown it being exercised? [14:54] bac: hmm… writing a test for this is harder than I thought. I demonstrated the debug output manually by running a script with -vvv but that'd be a bit of a splurge for testing something so basic. [14:55] I don't see any way to inject a fake database connection. There may be a way to inject a fake logger, but I don't know how. [14:56] (But FWIW yes I did see it work and it was just the thing I wished I'd had so many times before) [14:59] jtv: ok. i just wanted to ensure it had been exercised at least a little, to ensure the formatting string gets populated correctly, no syntax errors, etc === matsubara-lunch is now known as matsubara === bac changed the topic of #launchpad-reviews to: On call: adeuring,bac || Reviewing: -, || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:10] bac: a very valid concern—I've had breakage in the past because of silly mistakes in composing exception messages. Thanks. === deryck is now known as deryck[lunch] [17:23] adeuring: I added another test to the branch you reviewed, and I refactored the tests that you mentioned. [17:23] https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-638924-milestone-page-eager-loading/+merge/42586 [17:24] EdwinGrubbs: cool -- though I think branch was fine before :) I'll have a look [17:31] EdwinGrubbs: still r=me, just one minor nitpick [17:32] thanks === adeuring changed the topic of #launchpad-reviews to: On call: bac || Reviewing: -, || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === deryck[lunch] is now known as deryck === benji is now known as benji-lunch === benji-lunch is now known as benji === matsubara is now known as matsubara-afk === bac 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