=== matsubara is now known as matsubara-afk [03:38] lifeless: You might be interested in reviewing https://code.edge.https://code.edge.launchpad.net/~stub/launchpad/cronscripts/+merge/36005 -- cronscript oops stuff we discussed and some delinting noise. [03:39] what was LibrarianFormatter for ? [03:49] lifeless: Instead of spitting out exceptions to stderr, it stored them in the Librarian and spat out a URL. For a while some of the batch jobs where insanely noisy and this made them slightly more readable. [03:50] kk [03:51] CrashScript has huge vertical whitespace [03:52] could you compact it a little? [03:53] k [03:55] I only ask cause I couldn't really read it without my eyes rolling back in their sockets [04:18] lifeless: there is also https://code.edge.launchpad.net/~thumper/launchpad/branch-distro-set-old-mature/+merge/36095 if you'd like to approve another trivial branch [04:20] line 30, the extra vws weirds me [04:20] up to you [04:31] lifeless: are you vws phobic? [04:32] * wgrant likes LibrarianFormatter. [04:32] It's handy until Soyuz gets fixed. [04:32] thumper: no, but it often makes things harder to read [04:32] thumper: my rule of thumb is that if I need vws I need a new function [04:32] thumper: but its a shallow thing - I don't fundamentally care; I'll make it look how I prefer when I touch something, but really, anything goes. === Ursinha is now known as Ursinha-zzz [04:40] :) [04:52] wgrant: I can leave it if it is useful. I didn't think anyone liked it any more. [04:53] stub: It's still useful for people to see it in Soyuz upload logs. [04:53] Since Soyuz can be a bit harsh sometimes. [04:55] https://bugs.edge.launchpad.net/launchpad-foundations/+bug/641103 [04:55] <_mup_> Bug #641103: LibrarianFormatter should die [06:14] mwhudson: interested in the branch-distro script change to avoid the scan/ [06:14] ? [06:14] mwhudson: it is surprisingly small [06:15] thumper: sure [06:16] * thumper gets it ready [06:17] * thumper sighs [06:17] other test failures [06:17] due to my groovey new code [06:17] which now assumes real database data [06:18] heh [06:18] there are some rather extreme fakes in the tests for branch-distro iirc [06:19] * thumper is rerunning the tests [06:19] * thumper forgot -v [06:20] it is snowing again [06:20] ! [06:20] very pleasant day in wellington today [06:20] w00t [06:20] all passing again [06:24] mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/branch-distro-avoid-scan/+merge/36103 [06:26] mwhudson: I'm going to help with dinner, feel free to leave comments, and I'll look later [06:27] thumper: ok === StevenK changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === noodles775 changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [StevenK, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === bigjools is now known as bigjools-afk === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [StevenK, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [10:43] On call: gmb || Reviewing: noodles775 || queue: [bryceharrington] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === bryceh changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: noodles775 || queue: [bryceharrington] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [10:47] Thanks gmb [10:48] noodles775, r=me; one change needed and one slight refactoring requested. [10:48] Thanks gmb, I'll take a look. === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: bryceh || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [10:49] bryceh, lp-617679-code, right? [11:11] gmb: with makeLaunchpadService(), why not just put it in TestCaseWithFactory? (ie. is there a reason why I should have to think about a separate subclass etc. when adding an API test?) [11:12] noodles775, Well, if you added a WebServiceTestCase, you could also set its layer to AppServiceLayer by default. Other than that, though, there's no reason not to just put it in *WithFactory. [11:12] gmb: why do you need a WebServiceTestCase [11:13] lifeless, You don't. It would save people from having to re-do what noodles775 did in one of his test cases for the webservice, but it was just an idea. As noodles775 said, might as well add his makeLaunchpadService() method to TestCaseWithFactory,. [11:14] I haven't seen the code, but unless it also uses factory methods, why not just have it standalone ? [11:16] That would also work. noodles775, can you confirm that that's doable? [11:17] It does use factory.makePerson currently. [11:20] But sure, it could go in lp.testing._webservice.py [11:20] if its using factory.makePerson it should be on the factory [11:23] D'OH. [11:24] I blame lack of caffeine for my not spotting the blindingly obvious. === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [11:37] noodles775, So, as has been said, that method should be on the factory, not on some other TestCase. [11:37] Yep, I'll move it to the factory. [11:39] Cool. [11:39] * gmb grabs a drink; bbiab === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: thumper || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mrevell is now known as mrevell-lunch === gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mars changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: lunch, Ursinha || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === bigjools-afk is now known as bigjools === mrevell-lunch is now known as mrevell === gmb changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: -, Ursinha || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:00] Morning mars [14:00] Morning gmb! [14:38] mars, I have one for your queue. https://code.edge.launchpad.net/~gmb/launchpad/bug-644346/+merge/36143 === gmb changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: -, Ursinha || queue: [gmb] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:38] Nice and small. [14:38] gmb, thanks === jcsackett changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: -, Ursinha || queue: [gmb, jcsackett] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:58] hello, mars and gmb. [14:58] https://code.edge.launchpad.net/~jcsackett/launchpad/series-need-usage-attributes-643902/+merge/36145 could use a review, when you have time. [14:59] jcsackett, I'll take a look shorly. [14:59] *shortly [14:59] jcsackett, I have two in the queue before that [14:59] thanks gmb [14:59] gmb, thanks. :-) === gmb changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: jcsackett, Ursinha || queue: [gmb] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:59] * gmb grabs a drink first [15:10] jcsackett, On line 177 you say "# If translations_usage is set for the Product, respect it" but then you reference self.distribution.translations_usage. I'm assuming you need to s/Product/Distribution in the comment. [15:11] gmb: you're correct. making the change. [15:11] Same on line 241 [15:12] jcsackett, You need to add some comments or docstrings to the start of your tests explaining what they test (you should phrase this as a statement of expected behaviour, e.g. "The frobnob goes boing." rather than "Test that the frobnob goes boing." [15:13] Other than that, r=me. === gmb changed the topic of #launchpad-reviews to: On call: gmb, mars || Reviewing: -, Ursinha || queue: [gmb] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:14] gmb: you got it; i'll make those changes. [15:14] thanks a lot! === leonardr_ is now known as leonardr [16:06] rockstar, could you review https://code.edge.launchpad.net/~abentley/launchpad/incremental-diff-driveby/+merge/36156 please? [16:08] abentley, sure. [16:08] rockstar, thanks. [16:09] abentley, is any of that lint legitimate? [16:09] rockstar, I don't think so, but I'm open to other opinions. [16:09] abentley, it looks like the first one, line 715 of ./lib/lp/code/interfaces/branchmergeproposal.py, might be. === gmb changed the topic of #launchpad-reviews to: On call: mars || Reviewing: -, Ursinha || queue: [gmb] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:11] rockstar, AFAICT, there are two blank lines above notify_modified. [16:11] abentley, yeah, I looked. WTF. [16:12] rockstar, I think it's because the comment is there, but if that's the reason, it should say "found 0". === benji is now known as benji-lunch [17:26] danilos: https://code.edge.launchpad.net/~henninge/launchpad/recife-poimport/+merge/36165 [17:27] henninge, thanks, can you please find someone else as a reviewer if you are in a hurry to get some rest because I am chasing that nasty bug you filed :) [17:28] danilos: ok [17:28] danilos: that is a good excuse ;-) [17:29] unfortunately it's oversized === salgado is now known as salgado-lunch === gary_poster is now known as gary-lunch === gmb changed the topic of #launchpad-reviews to: On call: mars || Reviewing: Ursinha || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-lunch === Ursinha is now known as Ursinha-lunch === sinzui changed the topic of #launchpad-reviews to: On call: mars || Reviewing: Ursinha || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [18:10] mars I added a large branch to your queue. While most the the branch are deletes, It still looks scary. I can get EdwinGrubbs or bac to do it tomorrow if you do not have time [18:20] sinzui, I'll take it === benji-lunch is now known as benji === mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === deryck is now known as deryck[lunch] [19:02] deryck[lunch], ping when you return === gary-lunch is now known as gary_poster === matsubara-lunch is now known as matsubara === Ursinha-lunch is now known as Ursinha === mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === salgado-lunch is now known as salgado === deryck[lunch] is now known as deryck [19:40] hi mars [19:41] hi deryck [19:42] deryck, I have a review here from mbp where he asks for a careful review and some advice on testing his mail handling code. [19:42] deryck, I was wondering if someone on your team would have knowledge in that domain that could help Martin out? [19:43] deryck, it is some work he did on the DKIM code: https://code.edge.launchpad.net/~mbp/launchpad/dkim/+merge/35985 [19:43] mars, perhaps gmb or allenap. But the code team should have some expertise there, too. abentley wrote BaseMailer, I believe. [19:44] deryck, great. thank you for the help [19:44] np === mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [jml] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [19:56] StevenK, I am going to save jml's review for you. It moves code, so no need to balk at the line-count :) === mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [jml] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [[open],jml] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [jml] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:08] rockstar, could you please review https://code.edge.launchpad.net/~abentley/launchpad/safe-delete-recipe-build/+merge/36195 ? I think you'll like it. [20:42] abentley, sure. [21:09] rockstar, how about this? https://code.edge.launchpad.net/~abentley/launchpad/upload-failure-email-log/+merge/36205 [21:35] rockstar, mercy buckups. [21:35] abentley, no problem. Thanks for fixing that. [21:59] thumper: sorry for not reviewing branch-distro-avoid-scan yesterday, i don't know how i managed to forget about it [21:59] mwhudson: np, it isn't super urgent [21:59] not critical that is [22:03] thumper, incremental diff screenshot: http://people.canonical.com/~abentley/Screenshot-5.png [22:04] abentley: do we record the (from, to) revno pairs/ [22:04] abentley: it'd be nice to see a heading on that block [22:05] perhaps saying something like "Incremental diff from revno 123 - 130" [22:05] maybe [22:05] thumper, no, we don't record revnos. We record the revisions. [22:05] yay, another reason for the branch revision table [22:05] abentley: skype? [22:06] thumper, sure. [22:07] abentley: no headphones === mars 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 === matsubara is now known as matsubara-afk === salgado is now known as salgado-afk