[03:38] <stub> 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] <lifeless> what was  LibrarianFormatter for ?
[03:49] <stub> 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] <lifeless> kk
[03:51] <lifeless> CrashScript has huge vertical whitespace
[03:52] <lifeless> could you compact it a little?
[03:53] <stub> k
[03:55] <lifeless> I only ask cause I couldn't really read it without my eyes rolling back in their sockets
[04:18] <thumper> 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] <lifeless> line 30, the extra vws weirds me
[04:20] <lifeless> up to you
[04:31] <thumper> lifeless: are you vws phobic?
[04:32]  * wgrant likes LibrarianFormatter.
[04:32] <wgrant> It's handy until Soyuz gets fixed.
[04:32] <lifeless> thumper: no, but it often makes things harder to read
[04:32] <lifeless> thumper: my rule of thumb is that if I need vws I need a new function
[04:32] <lifeless> 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.
[04:40] <thumper> :)
[04:52] <stub> wgrant: I can leave it if it is useful. I didn't think anyone liked it any more.
[04:53] <wgrant> stub: It's still useful for people to see it in Soyuz upload logs.
[04:53] <wgrant> Since Soyuz can be a bit harsh sometimes.
[04:55] <stub> https://bugs.edge.launchpad.net/launchpad-foundations/+bug/641103
[04:55] <_mup_> Bug #641103: LibrarianFormatter should die <Launchpad Foundations:Triaged> <https://launchpad.net/bugs/641103>
[06:14] <thumper> mwhudson: interested in the branch-distro script change to avoid the scan/
[06:14] <thumper> ?
[06:14] <thumper> mwhudson: it is surprisingly small
[06:15] <mwhudson> thumper: sure
[06:16]  * thumper gets it ready
[06:17]  * thumper sighs
[06:17] <thumper> other test failures
[06:17] <thumper> due to my groovey new code
[06:17] <thumper> which now assumes real database data
[06:18] <mwhudson> heh
[06:18] <mwhudson> 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] <thumper> it is snowing again
[06:20] <mwhudson> !
[06:20] <mwhudson> very pleasant day in wellington today
[06:20] <thumper> w00t
[06:20] <thumper> all passing again
[06:24] <thumper> mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/branch-distro-avoid-scan/+merge/36103
[06:26] <thumper> mwhudson: I'm going to help with dinner, feel free to leave comments, and I'll look later
[06:27] <mwhudson> thumper: ok
[10:43] <bryceh>  On call: gmb || Reviewing: noodles775 || queue: [bryceharrington]  || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[10:47] <noodles775> Thanks gmb
[10:48] <gmb> noodles775, r=me; one change needed and one slight refactoring requested.
[10:48] <noodles775> Thanks gmb, I'll take a look.
[10:49] <gmb> bryceh, lp-617679-code, right?
[11:11] <noodles775> 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] <gmb> 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] <lifeless> gmb: why do you need a WebServiceTestCase
[11:13] <gmb> 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] <lifeless> I haven't seen the code, but unless it also uses factory methods, why not just have it standalone ?
[11:16] <gmb> That would also work. noodles775, can you confirm that that's doable?
[11:17] <noodles775> It does use factory.makePerson currently.
[11:20] <noodles775> But sure, it could go in lp.testing._webservice.py
[11:20] <lifeless> if its using factory.makePerson it should be on the factory
[11:23] <gmb> D'OH.
[11:24] <gmb> I blame lack of caffeine for my not spotting the blindingly obvious.
[11:37] <gmb> noodles775, So, as has been said, that method should be on the factory, not on some other TestCase.
[11:37] <noodles775> Yep, I'll move it to the factory.
[11:39] <gmb> Cool.
[11:39]  * gmb grabs a drink; bbiab
[14:00] <gmb> Morning mars
[14:00] <mars> Morning gmb!
[14:38] <gmb> mars, I have one for your queue. https://code.edge.launchpad.net/~gmb/launchpad/bug-644346/+merge/36143
[14:38] <gmb> Nice and small.
[14:38] <mars> gmb, thanks
[14:58] <jcsackett> hello, mars and gmb.
[14:58] <jcsackett> https://code.edge.launchpad.net/~jcsackett/launchpad/series-need-usage-attributes-643902/+merge/36145 could use a review, when you have time.
[14:59] <gmb> jcsackett, I'll take a look shorly.
[14:59] <gmb> *shortly
[14:59] <mars> jcsackett, I have two in the queue before that
[14:59] <mars> thanks gmb
[14:59] <jcsackett> gmb, thanks. :-)
[14:59]  * gmb grabs a drink first
[15:10] <gmb> 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] <jcsackett> gmb: you're correct. making the change.
[15:11] <gmb> Same on line 241
[15:12] <gmb> 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] <gmb> Other than that, r=me.
[15:14] <jcsackett> gmb: you got it; i'll make those changes.
[15:14] <jcsackett> thanks a lot!
[16:06] <abentley> rockstar, could you review https://code.edge.launchpad.net/~abentley/launchpad/incremental-diff-driveby/+merge/36156 please?
[16:08] <rockstar> abentley, sure.
[16:08] <abentley> rockstar, thanks.
[16:09] <rockstar> abentley, is any of that lint legitimate?
[16:09] <abentley> rockstar, I don't think so, but I'm open to other opinions.
[16:09] <rockstar> abentley, it looks like the first one, line 715 of ./lib/lp/code/interfaces/branchmergeproposal.py, might be.
[16:11] <abentley> rockstar, AFAICT, there are two blank lines above notify_modified.
[16:11] <rockstar> abentley, yeah, I looked. WTF.
[16:12] <abentley> rockstar, I think it's because the comment is there, but if that's the reason, it should say "found 0".
[17:26] <henninge> danilos: https://code.edge.launchpad.net/~henninge/launchpad/recife-poimport/+merge/36165
[17:27] <danilos> 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] <henninge> danilos: ok
[17:28] <henninge> danilos: that is a good excuse ;-)
[17:29] <henninge> unfortunately it's oversized
[18:10] <sinzui> 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] <mars> sinzui, I'll take it
[19:02] <mars> deryck[lunch], ping when you return
[19:40] <deryck> hi mars
[19:41] <mars> hi deryck
[19:42] <mars> 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] <mars> deryck, I was wondering if someone on your team would have knowledge in that domain that could help Martin out?
[19:43] <mars> deryck, it is some work he did on the DKIM code: https://code.edge.launchpad.net/~mbp/launchpad/dkim/+merge/35985
[19:43] <deryck> mars, perhaps gmb or allenap.  But the code team should have some expertise there, too. abentley wrote BaseMailer, I believe.
[19:44] <mars> deryck, great. thank you for the help
[19:44] <deryck> np
[19:56] <mars> StevenK, I am going to save jml's review for you.  It moves code, so no need to balk at the line-count :)
[20:08] <abentley> 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] <rockstar> abentley, sure.
[21:09] <abentley> rockstar, how about this? https://code.edge.launchpad.net/~abentley/launchpad/upload-failure-email-log/+merge/36205
[21:35] <abentley> rockstar, mercy buckups.
[21:35] <rockstar> abentley, no problem.  Thanks for fixing that.
[21:59] <mwhudson> thumper: sorry for not reviewing branch-distro-avoid-scan yesterday, i don't know how i managed to forget about it
[21:59] <thumper> mwhudson: np, it isn't super urgent
[21:59] <thumper> not critical that is
[22:03] <abentley> thumper, incremental diff screenshot: http://people.canonical.com/~abentley/Screenshot-5.png
[22:04] <thumper> abentley: do we record the (from, to) revno pairs/
[22:04] <thumper> abentley: it'd be nice to see a heading on that block
[22:05] <thumper> perhaps saying something like "Incremental diff from revno 123 - 130"
[22:05] <thumper> maybe
[22:05] <abentley> thumper, no, we don't record revnos.  We record the revisions.
[22:05] <thumper> yay, another reason for the branch revision table
[22:05] <thumper> abentley: skype?
[22:06] <abentley> thumper, sure.
[22:07] <thumper> abentley: no headphones