| wallyworld | StevenK: want another code review to do? | 04:13 |
|---|---|---|
| StevenK | wallyworld: No? :-) | 04:13 |
| wallyworld | StevenK: that's ok. i thought i'd offer it to you first | 04:13 |
| StevenK | Actually, let me ask a different question. "Do I have a choice?" :-) | 04:13 |
| StevenK | wallyworld: But yes, I'll take a look | 04:14 |
| wallyworld | 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 |
| wallyworld | StevenK: if you want :-) https://code.edge.launchpad.net/~wallyworld/launchpad/recipe-find-related-branches/+merge/42097 | 04:14 |
| wallyworld | StevenK: feel free to pass if you want. i won't be offended :-) | 04:15 |
| StevenK | wallyworld: You mention a bug in the summary, but you don't link it to the branch? :-) | 04:17 |
| StevenK | wallyworld: I'm only surprised you have to kick the security proxy out of the way so much | 04:20 |
| wallyworld | StevenK: linked now | 04:27 |
| wallyworld | 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:28 |
| StevenK | wallyworld: Perhaps this means the zcml should be tweaked instead of telling zope to get out of your way? | 04:58 |
| wallyworld | StevenK: yes, perhaps. i can take another look | 04:59 |
| StevenK | wallyworld: Anyway, +1'd, but that would be cool | 04:59 |
| wallyworld | 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:00 |
| wallyworld | thanks for the review btw | 05:01 |
| === 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 | ||
| wgrant | adeuring: Can I throw a review your way? | 11:45 |
| adeuring | wgrant: sure; I'm currently reviewing an MP from EdwinGrubbs, but i'm nearly finished | 11:46 |
| === 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 | ||
| wgrant | https://code.launchpad.net/~wgrant/launchpad/bug-675621-packages-binary-scaling/+merge/42607 is the MP. | 11:47 |
| === 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 | ||
| * adeuring needs to remember that C-Y does not paste stuff from the clipboard in most programs | 12:03 | |
| adeuring | wgrant: r=me | 12:22 |
| === 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 | ||
| wgrant | adeuring: Thanks! | 12:23 |
| wgrant | adeuring: Could you please throw it at EC2? | 12:23 |
| adeuring | wgrant: sure | 12:23 |
| gmb | 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 | 12:42 |
| === mrevell is now known as mrevell-lunch | ||
| adeuring | gmb: sure | 12:58 |
| gmb | Thanks | 12:59 |
| adeuring | gmb: r=me | 13:15 |
| gmb | adeuring: Thanks. | 13:17 |
| === 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 | ||
| jtv | 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 | 13:32 |
| === vednis is now known as mars | ||
| === mrevell-lunch is now known as mrevell | ||
| bac | hi adeuring. much happening today? | 13:53 |
| adeuring | bac: well, three reviews so far | 13:53 |
| bac | adeuring: hopping! | 13:53 |
| bac | jtv: i'll look at it | 13:57 |
| jtv | thanks bac | 13:57 |
| === 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 | ||
| bac | jtv: are there any tests where you could demonstrate this loggin? | 14:33 |
| jtv | bac: I could probably arrange for one. | 14:34 |
| jtv | Wasn't sure it was needed, since this will be exercised all the time anyway. | 14:34 |
| bac | have you shown it being exercised? | 14:35 |
| jtv | 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:54 |
| jtv | 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:55 |
| jtv | (But FWIW yes I did see it work and it was just the thing I wished I'd had so many times before) | 14:56 |
| bac | 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 | 14:59 |
| === 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 | ||
| jtv | bac: a very valid concern—I've had breakage in the past because of silly mistakes in composing exception messages. Thanks. | 16:10 |
| === deryck is now known as deryck[lunch] | ||
| EdwinGrubbs | adeuring: I added another test to the branch you reviewed, and I refactored the tests that you mentioned. | 17:23 |
| EdwinGrubbs | https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-638924-milestone-page-eager-loading/+merge/42586 | 17:23 |
| adeuring | EdwinGrubbs: cool -- though I think branch was fine before :) I'll have a look | 17:24 |
| adeuring | EdwinGrubbs: still r=me, just one minor nitpick | 17:31 |
| EdwinGrubbs | thanks | 17:32 |
| === 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 | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!