/srv/irclogs.ubuntu.com/2010/07/23/#launchpad-reviews.txt

thumperanyone? https://code.edge.launchpad.net/~thumper/launchpad/recipe-owner/+merge/3074005:24
thumpermwhudson: ^^ maybe?05:28
thumpermwhudson: it is pretty simple05:28
mwhudsonthumper: you could verify the owner by examining the database object, maybe?05:30
thumpermwhudson: not in the browser test05:30
thumperwell05:30
thumperI could I suppose05:30
mwhudsonthumper: why not?  you create a branch database-style05:30
* thumper thinks of steps05:30
thumperhave to login, and get recipe for person05:31
thumpershouldn't be too hard05:31
thumperbetter option you think/05:31
thumper?05:31
mwhudsonit makes it more obvious what you're actually testing i think05:31
thumpermwhudson: http://pastebin.ubuntu.com/467815/05:42
thumperand I'm happy that utilities/paste seems to work again05:42
mwhudsonthumper: looks good05:43
thumperI do feel slightly that the test is testing two things05:44
thumperone that the teams are shown (which is kinda defined by the widget)05:44
thumperand the other is that we use the owner specified05:44
mwhudsoni guess you could do a more explicit test of the former05:44
mwhudsonoh05:45
* mwhudson opens his eyes05:45
mwhudsonyeah, maybe that should be two tests05:45
* thumper hacks05:46
thumpermwhudson: http://pastebin.ubuntu.com/467818/05:48
thumperlook ok?05:48
mwhudsonthumper: yes05:53
thumpergood05:53
thumper'cause I just typed ec2 land05:54
thumper:)05:54
mwhudsonheh05:54
mwhudsoni was busy writing awful code for lexbuilder05:54
thumperlexbuilder?05:55
lifelessmwhudson: is that open yet?05:55
mwhudsonlifeless: no, going through the management approval process aiui05:56
mwhudsonthumper: cody's image building service thing05:56
jtvOn call: -  || reviewing:  || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews12:14
danilosjtv, 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
jtvdanilos: hi!13:32
jtvuhhh13:32
jtv"I'm sprinting, sorry"  ;-)13:33
jtvNah, coming.  :)13:33
danilosjtv, no worries, it's only ~1100 lines or so :)13:33
jtvdanilos: gah13:33
=== 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
bacmorning13:47
=== 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
jtvbac: thanks... I've got several, but your choice which you take14:10
baclooking at bug 60067314:10
_mup_Bug #600673: Template selector on template approval form <import-queue> <ui> <Launchpad Translations:In Progress by jtv> <https://launchpad.net/bugs/600673>14:10
bacjtv: you've got a note in BRANCH.TODO.  was that done?14:11
jtvbac: great!  That's been stuck in "$%&#@ windmill won't work" for ages14:11
* jtv checks14:11
jtvbac: whoops, no!14:12
jtvbac: I'll retract that MP for now and fix that.  :-(14:12
bacwow, that was fast!14:12
jtvGood thing I made a note there... AIUI such a note prohibits landing.14:12
bacyep14:12
bacbig red flag to reviewers, too14:12
jtv(So good thing you spotted it, but also good to know it wasn't our last line of defense)14:12
james_whi bac, a small branch for your perusal if you would: https://code.edge.launchpad.net/~james-w/launchpad/devel/+merge/3077614:56
=== 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
bacjames_w: righto14:56
=== 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
bacjtv: your export-reorg branch has a failing test:  pofile.txt15:05
jtvbac: just not my week, is it...15:06
jtvI thought I'd run all tests.  :(15:06
baclooks easy15:06
jtvbac: I'll have to get to it later...  juggling too many balls right now15:12
bacjtv:  ok, i'll still finish up the review15:13
jtvbac: thanks15:13
=== matsubara is now known as matsubara-lunch
jtvbac: you were right, looks easy15:30
jtvforgot to pass some parameters15:30
jtvexportTranslationFiles accepts them but fails to pass them on as intended.15:32
jtvbac: export-reorg is fixed, passed all tests I cared to throw at it.15:36
bacthanks15:36
salgadojtv, did you file that bug on our windmill login helper?15:39
jtvsalgado: no...  something I wanted to ask: going to the page first and _then_ logging in looks like it shouldn't work at all.15:39
salgadojtv, but the test passes if you do that?15:40
jtvsalgado: I changed it like you said—ensure login, then open page.  And that worked.15:40
salgadojtv, and why you think it shouldn't work?15:40
jtvSo ISTM it's a bug in the test.  Just don't get why buildbot doesn't scream bloody murder.15:41
jtvsalgado: because you need to be logged in before you can open that page.15:41
salgadobut if it shouldn't work then the login helper should fail loudly rather than silently. ;)15:42
jtvWell AFAICS the login would work, but just too late for the test to get a working page.15:43
salgadothe login should work and redirect back to the page that was open when it started15:44
salgadoso the test would be able to proceed15:45
jtvoic15:47
jtvthen I'll file that bug15:47
jtvsalgado: bug 60919015:52
_mup_Bug #609190: Login problem in windmill test <Launchpad itself:New> <https://launchpad.net/bugs/609190>15:52
salgadojtv, cool, thanks15:52
=== 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
jtvbac: thanks...  I just fixed up my other branch as well.16:20
jtvI hope.16:20
jtvBut let me test that first.  :-)16:20
jelmerbac: Can I add a trivial qa fix branch to your queue?16:38
jtvbac: done fixing up both my broken branches—please consider them reviewable again16:58
jtvahem... one was already done of course16:59
* jtv eods17:06
=== gary-lunch is now known as gary_poster
abentleyrockstar, could you please review https://code.edge.launchpad.net/~abentley/launchpad/force-aptitude/+merge/30795 ?17:16
rockstarabentley, absolutely.17:16
jelmerAnybody around to do a quick qa fix review (6 line change + 7 lines cleanup) ?17:18
rockstarabentley, line 26-27 of the diff, why are you printing "Printing recipe:" and the recipe on separate lines?17:22
abentleyrockstar, 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:23
rockstarabentley, okay.17:24
abentleyrockstar, this log gives an example of what it looks like: http://librarian.dogfood.launchpad.net/51750418/buildlog.txt.gz17:25
=== matsubara-lunch is now known as matsubara
rockstarabentley, ah, much better!17:29
rockstarabentley, can you rubberstamp a merge for bzr-builder and bzr-builddeb trunk merges into sourcecode for me?18:02
=== 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
bacsinzui: where is your branch to review?18:56
bacMP?18:56
=== jelmer_ is now known as jelmer
* sinzui look18:58
jelmerhi Brad18:58
jelmerbac: Can I add a trivial qa fix branch to your queue?18:58
bacjelmer: sure18:58
=== 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
bacjelmer: is it on +activereviews?  you have two there18:58
sinzuibac: https://code.edge.launchpad.net/~sinzui/launchpad/launchpad-header-0/+merge/3057018:59
jelmerbac: Thanks! It's https://code.edge.launchpad.net/~jelmer/launchpad/600153-qafix/+merge/3079018:59
bacsinzui: ah, robert claimed it and then abstained.  that's why it doesn't show up18:59
rockstarabentley, can I use you as rubberstamp for bzr-builder update of sourcecode?19:30
abentleyrockstar, rs=me.19:30
abentleyrockstar, sorry, was on lunch.19:31
rockstarabentley, ta19:31
rockstarabentley, no problem.  You're allowed to eat.  :)19:31
abentleyrockstar, could you please review https://code.edge.launchpad.net/~abentley/launchpad/package-build-recipe/+merge/30818 ?20:33
rockstarabentley, sure.20:34
rockstarabentley, it looks like there might be some conflicts.20:39
rockstar(At least, that's what the diff says.  It's sometimes wrong)20:39
abentleyrockstar, I merged just before pushing, so I'd be surprised.20:39
abentleyrockstar, the merge conflict is introduced in db-devel, which is why I didn't catch it.20:44
rockstarabentley, ah, okay.  The conflict looks like the result of a blanket lint fix, which makes me a bit sad.20:45
abentleyrockstar, plus I don't like the idea of remove_security_proxy_and_shout_at_engineer.20:47
abentleyrockstar, updated version pushed.20:48
sinzuirockstar, are you available to do a UI review today?21:02
rockstarsinzui, I am after I am finished with abentley, yes.21:03
sinzuiI will then make a formal request on my MP21:03
rockstarsinzui, cool.21:04
rockstarabentley, why do you catch ProgrammingError and then raise without an argument?21:06
abentleyrockstar, 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
rockstarabentley, ah, is this the problem you were bumping into this morning?21:07
abentleyrockstar, I raise without an argument because I want to see the traceback from where it was originally raised.21:07
abentleyrockstar, yes.21:08
rockstarabentley, oka.21:08
rockstarabentley, I wonder if a comment to that effect might be good.21:08
rockstarabentley, actually, no, probably not.21:08
bacjelmer: i just got off a long call and will do your review now.  sorry for the delay21:09
=== 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
rockstarabentley, I don't see any tests for lfaUrl, log_url, or upload_log_url.  Are those abstracted somewhere?21:13
abentleyrockstar, I shouldn't have to test those, because they're provided by packagebuild.21:15
abentleyor buildfarmjob for log_url.21:15
rockstarabentley, okay, lemme actually fetch the branch.  The diff seems misleading then.21:16
rockstarabentley, it looks like lfaURL is part of SourcePackageRecipeBuild21:19
abentleyrockstar, Yes, there are tests for that one.21:22
abentleyrockstar, that's basically a view helper.21:22
rockstarabentley, can you point me to the tests for it?21:22
abentleyrockstar, it makes +files/upload-foo.log work.21:22
rockstarabentley, okay, so it's not unit tested, but the functionality is being tested.21:25
abentleyrockstar, right.21:25
rockstarabentley, I guess that's okay, although I'd be more comfortable with a unit test as well.21:26
rockstarI leave that to your discretion.21:26
abentleyrockstar, I only added it because there were failing tests.21:26
* rockstar nods.21:27
rockstarabentley, 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:38
abentleyrockstar, you're not planning on landing in in db-devel, are you?21:39
rockstarabentley, no, but I need to coordinate the merge from devel to db-devel.21:39
abentleyrockstar, as you wish.21:40
rockstarSo I need to land in devel, then merge my patch into db-devel and make sure the change is made.21:40
abentleyrockstar, I'm doing a full test suite run now.21:40
abentleyrockstar, I'll probably land it this weekend.21:40
abentleyrockstar, I'd like to try calling you on Empathy chat.  Cool?21:41
rockstarabentley, lemme figure out how to make Empathy happy with my headset.  One sec.21:42
rockstarabentley, okay, call away.21:43
abentleyrockstar, nevermind.  Let's do mumble as usual.21:44
rockstarabentley, okay, cool.21:44
=== 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

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!