| === matsubara is now known as matsubara-afk | ||
| 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:38 |
|---|---|---|
| lifeless | what was LibrarianFormatter for ? | 03:39 |
| 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:49 |
| lifeless | kk | 03:50 |
| lifeless | CrashScript has huge vertical whitespace | 03:51 |
| lifeless | could you compact it a little? | 03:52 |
| stub | k | 03:53 |
| lifeless | I only ask cause I couldn't really read it without my eyes rolling back in their sockets | 03:55 |
| 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:18 |
| lifeless | line 30, the extra vws weirds me | 04:20 |
| lifeless | up to you | 04:20 |
| thumper | lifeless: are you vws phobic? | 04:31 |
| * 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:32 |
| === Ursinha is now known as Ursinha-zzz | ||
| thumper | :) | 04:40 |
| stub | wgrant: I can leave it if it is useful. I didn't think anyone liked it any more. | 04:52 |
| 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:53 |
| 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> | 04:55 |
| 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:14 |
| mwhudson | thumper: sure | 06:15 |
| * thumper gets it ready | 06:16 | |
| * 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:17 |
| mwhudson | heh | 06:18 |
| mwhudson | there are some rather extreme fakes in the tests for branch-distro iirc | 06:18 |
| * thumper is rerunning the tests | 06:19 | |
| * thumper forgot -v | 06:19 | |
| 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:20 |
| thumper | mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/branch-distro-avoid-scan/+merge/36103 | 06:24 |
| thumper | mwhudson: I'm going to help with dinner, feel free to leave comments, and I'll look later | 06:26 |
| mwhudson | thumper: ok | 06:27 |
| === 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 | ||
| bryceh | On call: gmb || Reviewing: noodles775 || queue: [bryceharrington] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | 10:43 |
| === 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 | ||
| noodles775 | Thanks gmb | 10:47 |
| gmb | noodles775, r=me; one change needed and one slight refactoring requested. | 10:48 |
| noodles775 | Thanks gmb, I'll take a look. | 10:48 |
| === 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 | ||
| gmb | bryceh, lp-617679-code, right? | 10:49 |
| 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:11 |
| 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:12 |
| 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:13 |
| lifeless | I haven't seen the code, but unless it also uses factory methods, why not just have it standalone ? | 11:14 |
| gmb | That would also work. noodles775, can you confirm that that's doable? | 11:16 |
| noodles775 | It does use factory.makePerson currently. | 11:17 |
| 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:20 |
| gmb | D'OH. | 11:23 |
| gmb | I blame lack of caffeine for my not spotting the blindingly obvious. | 11:24 |
| === 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 | ||
| 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:37 |
| gmb | Cool. | 11:39 |
| * gmb grabs a drink; bbiab | 11:39 | |
| === 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 | ||
| gmb | Morning mars | 14:00 |
| mars | Morning gmb! | 14:00 |
| gmb | mars, I have one for your queue. https://code.edge.launchpad.net/~gmb/launchpad/bug-644346/+merge/36143 | 14:38 |
| === 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 | ||
| gmb | Nice and small. | 14:38 |
| mars | gmb, thanks | 14:38 |
| === 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 | ||
| 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:58 |
| 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 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 | ||
| * gmb grabs a drink first | 14:59 | |
| 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:10 |
| jcsackett | gmb: you're correct. making the change. | 15:11 |
| gmb | Same on line 241 | 15:11 |
| 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:12 |
| gmb | Other than that, r=me. | 15:13 |
| === 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 | ||
| jcsackett | gmb: you got it; i'll make those changes. | 15:14 |
| jcsackett | thanks a lot! | 15:14 |
| === leonardr_ is now known as leonardr | ||
| abentley | rockstar, could you review https://code.edge.launchpad.net/~abentley/launchpad/incremental-diff-driveby/+merge/36156 please? | 16:06 |
| rockstar | abentley, sure. | 16:08 |
| abentley | rockstar, thanks. | 16:08 |
| 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:09 |
| === 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 | ||
| abentley | rockstar, AFAICT, there are two blank lines above notify_modified. | 16:11 |
| rockstar | abentley, yeah, I looked. WTF. | 16:11 |
| abentley | rockstar, I think it's because the comment is there, but if that's the reason, it should say "found 0". | 16:12 |
| === benji is now known as benji-lunch | ||
| henninge | danilos: https://code.edge.launchpad.net/~henninge/launchpad/recife-poimport/+merge/36165 | 17:26 |
| 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:27 |
| henninge | danilos: ok | 17:28 |
| henninge | danilos: that is a good excuse ;-) | 17:28 |
| henninge | unfortunately it's oversized | 17:29 |
| === 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 | ||
| 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:10 |
| mars | sinzui, I'll take it | 18:20 |
| === 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] | ||
| mars | deryck[lunch], ping when you return | 19:02 |
| === 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 | ||
| deryck | hi mars | 19:40 |
| mars | hi deryck | 19:41 |
| 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:42 |
| 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:43 |
| mars | deryck, great. thank you for the help | 19:44 |
| deryck | np | 19:44 |
| === 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 | ||
| mars | StevenK, I am going to save jml's review for you. It moves code, so no need to balk at the line-count :) | 19:56 |
| === 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 | ||
| 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:08 |
| rockstar | abentley, sure. | 20:42 |
| abentley | rockstar, how about this? https://code.edge.launchpad.net/~abentley/launchpad/upload-failure-email-log/+merge/36205 | 21:09 |
| abentley | rockstar, mercy buckups. | 21:35 |
| rockstar | abentley, no problem. Thanks for fixing that. | 21:35 |
| 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 | 21:59 |
| abentley | thumper, incremental diff screenshot: http://people.canonical.com/~abentley/Screenshot-5.png | 22:03 |
| 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:04 |
| 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:05 |
| abentley | thumper, sure. | 22:06 |
| thumper | abentley: no headphones | 22:07 |
| === 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 | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
