[05:24] anyone? https://code.edge.launchpad.net/~thumper/launchpad/recipe-owner/+merge/30740 [05:28] mwhudson: ^^ maybe? [05:28] mwhudson: it is pretty simple [05:30] thumper: you could verify the owner by examining the database object, maybe? [05:30] mwhudson: not in the browser test [05:30] well [05:30] I could I suppose [05:30] thumper: why not? you create a branch database-style [05:30] * thumper thinks of steps [05:31] have to login, and get recipe for person [05:31] shouldn't be too hard [05:31] better option you think/ [05:31] ? [05:31] it makes it more obvious what you're actually testing i think [05:42] mwhudson: http://pastebin.ubuntu.com/467815/ [05:42] and I'm happy that utilities/paste seems to work again [05:43] thumper: looks good [05:44] I do feel slightly that the test is testing two things [05:44] one that the teams are shown (which is kinda defined by the widget) [05:44] and the other is that we use the owner specified [05:44] i guess you could do a more explicit test of the former [05:45] oh [05:45] * mwhudson opens his eyes [05:45] yeah, maybe that should be two tests [05:46] * thumper hacks [05:48] mwhudson: http://pastebin.ubuntu.com/467818/ [05:48] look ok? [05:53] thumper: yes [05:53] good [05:54] 'cause I just typed ec2 land [05:54] :) [05:54] heh [05:54] i was busy writing awful code for lexbuilder [05:55] lexbuilder? [05:55] mwhudson: is that open yet? [05:56] lifeless: no, going through the management approval process aiui [05:56] thumper: cody's image building service thing [12:14] On call: - || reviewing: || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [13:32] jtv, hey, I reviewed a branch for you :) how about you review one for me... https://code.edge.launchpad.net/~danilo/launchpad/translatedlanguage/+merge/30760 ;) [13:32] danilos: hi! [13:32] uhhh [13:33] "I'm sprinting, sorry" ;-) [13:33] Nah, coming. :) [13:33] jtv, no worries, it's only ~1100 lines or so :) [13:33] danilos: gah === bac 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 [13:47] morning === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:10] bac: thanks... I've got several, but your choice which you take [14:10] looking at bug 600673 [14:10] <_mup_> Bug #600673: Template selector on template approval form [14:11] jtv: you've got a note in BRANCH.TODO. was that done? [14:11] bac: great! That's been stuck in "$%&#@ windmill won't work" for ages [14:11] * jtv checks [14:12] bac: whoops, no! [14:12] bac: I'll retract that MP for now and fix that. :-( [14:12] wow, that was fast! [14:12] Good thing I made a note there... AIUI such a note prohibits landing. [14:12] yep [14:12] big red flag to reviewers, too [14:12] (So good thing you spotted it, but also good to know it wasn't our last line of defense) [14:56] hi bac, a small branch for your perusal if you would: https://code.edge.launchpad.net/~james-w/launchpad/devel/+merge/30776 === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: jtv || queue: [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:56] james_w: righto === sinzui changed the topic of #launchpad-reviews to: On call: bac || reviewing: jtv || queue: [james_w, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:05] jtv: your export-reorg branch has a failing test: pofile.txt [15:06] bac: just not my week, is it... [15:06] I thought I'd run all tests. :( [15:06] looks easy [15:12] bac: I'll have to get to it later... juggling too many balls right now [15:13] jtv: ok, i'll still finish up the review [15:13] bac: thanks === matsubara is now known as matsubara-lunch [15:30] bac: you were right, looks easy [15:30] forgot to pass some parameters [15:32] exportTranslationFiles accepts them but fails to pass them on as intended. [15:36] bac: export-reorg is fixed, passed all tests I cared to throw at it. [15:36] thanks [15:39] jtv, did you file that bug on our windmill login helper? [15:39] salgado: no... something I wanted to ask: going to the page first and _then_ logging in looks like it shouldn't work at all. [15:40] jtv, but the test passes if you do that? [15:40] salgado: I changed it like you said—ensure login, then open page. And that worked. [15:40] jtv, and why you think it shouldn't work? [15:41] So ISTM it's a bug in the test. Just don't get why buildbot doesn't scream bloody murder. [15:41] salgado: because you need to be logged in before you can open that page. [15:42] but if it shouldn't work then the login helper should fail loudly rather than silently. ;) [15:43] Well AFAICS the login would work, but just too late for the test to get a working page. [15:44] the login should work and redirect back to the page that was open when it started [15:45] so the test would be able to proceed [15:47] oic [15:47] then I'll file that bug [15:52] salgado: bug 609190 [15:52] <_mup_> Bug #609190: Login problem in windmill test [15:52] jtv, cool, thanks === gary_poster is now known as gary-lunch === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: james_w || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:20] bac: thanks... I just fixed up my other branch as well. [16:20] I hope. [16:20] But let me test that first. :-) [16:38] bac: Can I add a trivial qa fix branch to your queue? [16:58] bac: done fixing up both my broken branches—please consider them reviewable again [16:59] ahem... one was already done of course [17:06] * jtv eods === gary-lunch is now known as gary_poster [17:16] rockstar, could you please review https://code.edge.launchpad.net/~abentley/launchpad/force-aptitude/+merge/30795 ? [17:16] abentley, absolutely. [17:18] Anybody around to do a quick qa fix review (6 line change + 7 lines cleanup) ? [17:22] abentley, line 26-27 of the diff, why are you printing "Printing recipe:" and the recipe on separate lines? [17:23] rockstar, I'm printing 'Building recipe:' so to help break up the recipe from the body of the log. I'm printing the recipe verbatim so that it's easy to copy and paste. [17:24] abentley, okay. [17:25] rockstar, this log gives an example of what it looks like: http://librarian.dogfood.launchpad.net/51750418/buildlog.txt.gz === matsubara-lunch is now known as matsubara [17:29] abentley, ah, much better! [18:02] abentley, can you rubberstamp a merge for bzr-builder and bzr-builddeb trunk merges into sourcecode for me? === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: sinzui || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [18:56] sinzui: where is your branch to review? [18:56] MP? === jelmer_ is now known as jelmer [18:58] * sinzui look [18:58] hi Brad [18:58] bac: Can I add a trivial qa fix branch to your queue? [18:58] jelmer: sure === jelmer changed the topic of #launchpad-reviews to: On call: bac || reviewing: sinzui || queue: [jelmer] ] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [18:58] jelmer: is it on +activereviews? you have two there [18:59] bac: https://code.edge.launchpad.net/~sinzui/launchpad/launchpad-header-0/+merge/30570 [18:59] bac: Thanks! It's https://code.edge.launchpad.net/~jelmer/launchpad/600153-qafix/+merge/30790 [18:59] sinzui: ah, robert claimed it and then abstained. that's why it doesn't show up [19:30] abentley, can I use you as rubberstamp for bzr-builder update of sourcecode? [19:30] rockstar, rs=me. [19:31] rockstar, sorry, was on lunch. [19:31] abentley, ta [19:31] abentley, no problem. You're allowed to eat. :) [20:33] rockstar, could you please review https://code.edge.launchpad.net/~abentley/launchpad/package-build-recipe/+merge/30818 ? [20:34] abentley, sure. [20:39] abentley, it looks like there might be some conflicts. [20:39] (At least, that's what the diff says. It's sometimes wrong) [20:39] rockstar, I merged just before pushing, so I'd be surprised. [20:44] rockstar, the merge conflict is introduced in db-devel, which is why I didn't catch it. [20:45] abentley, ah, okay. The conflict looks like the result of a blanket lint fix, which makes me a bit sad. [20:47] rockstar, plus I don't like the idea of remove_security_proxy_and_shout_at_engineer. [20:48] rockstar, updated version pushed. [21:02] rockstar, are you available to do a UI review today? [21:03] sinzui, I am after I am finished with abentley, yes. [21:03] I will then make a formal request on my MP [21:04] sinzui, cool. [21:06] abentley, why do you catch ProgrammingError and then raise without an argument? [21:07] rockstar, I want ProgrammingErrors to propagate normally, because it's confusing if that exception handler tries to swallow them, and it masks the original cause. [21:07] abentley, ah, is this the problem you were bumping into this morning? [21:07] rockstar, I raise without an argument because I want to see the traceback from where it was originally raised. [21:08] rockstar, yes. [21:08] abentley, oka. [21:08] abentley, I wonder if a comment to that effect might be good. [21:08] abentley, actually, no, probably not. [21:09] jelmer: i just got off a long call and will do your review now. sorry for the delay === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: jelmer || queue: [-] ] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [21:13] abentley, I don't see any tests for lfaUrl, log_url, or upload_log_url. Are those abstracted somewhere? [21:15] rockstar, I shouldn't have to test those, because they're provided by packagebuild. [21:15] or buildfarmjob for log_url. [21:16] abentley, okay, lemme actually fetch the branch. The diff seems misleading then. [21:19] abentley, it looks like lfaURL is part of SourcePackageRecipeBuild [21:22] rockstar, Yes, there are tests for that one. [21:22] rockstar, that's basically a view helper. [21:22] abentley, can you point me to the tests for it? [21:22] rockstar, it makes +files/upload-foo.log work. [21:25] abentley, okay, so it's not unit tested, but the functionality is being tested. [21:25] rockstar, right. [21:26] abentley, I guess that's okay, although I'd be more comfortable with a unit test as well. [21:26] I leave that to your discretion. [21:26] rockstar, I only added it because there were failing tests. [21:27] * rockstar nods. [21:38] abentley, okay, r=me. However, I'm going to have to wait for your branch to land before I change the Cancel/Rescore link, because it needs to hinge on the status now, and changing that in devel and then getting merged into db-devel after you land your patch will send us into testfix. [21:39] rockstar, you're not planning on landing in in db-devel, are you? [21:39] abentley, no, but I need to coordinate the merge from devel to db-devel. [21:40] rockstar, as you wish. [21:40] So I need to land in devel, then merge my patch into db-devel and make sure the change is made. [21:40] rockstar, I'm doing a full test suite run now. [21:40] rockstar, I'll probably land it this weekend. [21:41] rockstar, I'd like to try calling you on Empathy chat. Cool? [21:42] abentley, lemme figure out how to make Empathy happy with my headset. One sec. [21:43] abentley, okay, call away. [21:44] rockstar, nevermind. Let's do mumble as usual. [21:44] abentley, okay, cool. === gary_poster_ is now known as gary_poster === 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 === matsubara is now known as matsubara-afk