=== noodles775 changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [09:14] noodles775: for which branch do you need a review? [09:15] adeuring: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-get-or-create-from-identity/+merge/28885 [09:15] Thanks! [09:16] * adeuring is looking === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [09:16] I asked salgado to take a brief look yesterday (as it's his area), and he's commented, but it needs a proper review. [10:06] adeuring: also, i've got another branch that's not ready for review, but I need some help (related to private XMLRPC authentication - I'm a little unfamiliar) if you've got any input: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-priv-xmlrpc-access-getOrCreate/+merge/29059 [10:09] noodles775: so, you want to add another celebrity? [10:09] adeuring: that's already done (and landed by StevenK). At the moment I'm just trying to write a test that logs in as admin and passes. [10:11] The xmlrpc call works fine (I can switch to zope.Public and the tests work), but I can't see how to assign launchpad.Admin to the xmlrpc view (or if I even should be going down that direction - I can't see any other examples in the code) [10:12] Hrm, maybe the test should be authenticating differently, or the logged in user needs ssh keys etc. [10:12] noodles775: as I understand it, you just need to ensure in checkAuthenitcated that the current user is your celebrity. Or am I missing something? [10:13] adeuring: that's what I would have thought, but that breakpoint is never being called (and yet the tests return 401 Unauthorized) [10:14] noodles775: seems that some other authentication check is kicking in then? [10:14] Yep... I'll check if it's related to ssh keys on the test users. [10:14] is there any auth class for the same permission and interface class? [10:15] Nope - it's a new class in that branch. [10:21] noodles775: after looking closer, I must ammit that I have no clue why this doesn't work... [10:22] adeuring: ok, thanks for looking :D [10:33] adeuring: found it... I needed to supply the credentials when creating the xmlrpc server proxy... obvious now. [10:33] noodles775: uuhhh, yes ;) [10:34] But I still need to find out how I can ensure other users have launchpad.Admin on the view... [10:34] * noodles775 keeps looking. [11:04] noodles775: overall, your code looks good. Just one question: SHouldn't the "with MasterDBPolicy():" be moved from processPositiveAssrtion() to getOrCreateByOpenidIdentifier()? [11:06] adeuring: yes, you're right. [11:10] noodles775: OK, so r=me (including aonther nitpick about a typo) [11:11] Thanks adeuring [11:12] On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [11:24] adeuring: the other one is ready now if you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-priv-xmlrpc-access-getOrCreate/+merge/29059 [11:45] noodles775: I'll look === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [12:09] noodles775: again, a nice branch. But I think it is a bit odd to have a class PersonSetAPIView (ie.e.,a quite generic name) where the method getorCreateByOpenIDIdentifier() unconditionally providesthe rationale "software centerpurchase" as a account creation rationale. Shouldn't we either make the class name more specific or provide the rationale as a parameter of the method? [12:10] (or use a method name like getOrCreateForSoftwarePurchase()) [12:16] adeuring, can you review a oneliner (testfix) ? [12:16] rinze: sure [12:16] https://code.edge.launchpad.net/~jelmer/launchpad/fix-db-devel/+merge/29070 [12:18] a warning about the completeness of the dailybuilds feature was added recently, I suspect that broke it [12:19] rinze: well, the change looks straightforward, r=me. [12:19] adeuring: Thanks [12:45] adeuring: yes - I thought the same when I pushed the last rev. So I'd be keen to change the application name... most of the other private xmlrpc apps have names like 'ICodehostingApplication' identifying who uses them. [12:46] hello adeuring. busy review day? [12:46] bac: quite intersting branches from noodles775 :) [12:46] So perhaps, ISoftwareCenterAgentAPI, http://..../softwarecenteragent etc.? [12:47] noodles775: ok, so something like s/Person/Shopper/ ? [12:47] yes, i saw. looks like fun... [12:47] ...in the app name? [12:47] * bac relocates [12:48] The app name usually matches the API, so perhaps ISoftwareCenterAgentApplication... [12:50] noodles775: ah, right. so, that, combined with a method name like getOrCreateShopCustomer()? [12:50] Sounds good. [12:50] noodles775: ok, r=me [12:57] w 5 === matsubara-afk is now known as matsubara === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: noodles775 , -|| queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === rinze is now known as ctrlsoft === henninge_ is now known as henninge [13:55] jtv: your reply looks good but you did not push your changes. Did you rename "get_all_important_translations" ? [13:55] henninge: knew I forgot something! hang on [13:55] (Well, I didn't know you'd be this quick so technically I just hadn't gotten around to it yet) [13:56] jtv: I guess you'd like to be through with this before the game ... ;-) [13:56] jtv: also, since this branch only contains tests, it should not need a full test suite run, should it? === adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -, -|| queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [13:57] Thanks for reminding me of the game. :-) I started incredibly early today, so don't think I'll do much after. [13:58] henninge: that's the theory. Practice doesn't always follow theory. :) [13:58] jtv: one more branch after this and we can merge db-stable again? [13:59] henninge: yes, that's D.'s use-upstream-translations branch. [13:59] That will also enable us to run the "mirror" tests that I commented out. [14:00] henninge: oh, and I snuck something extra in: I defined flushing orders when disabling TMs [14:00] Push is underway. [14:01] Ah, once bitten ... [14:04] henninge: push is done, diff updated... [14:07] bac, got time for this one? https://code.launchpad.net/~jtv/launchpad/bug-181254/+merge/29056 [14:09] jtv: yes, please add to topic [14:10] bac: "reviewing" or "queue"? :-) === jtv changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -,jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:13] queue, of course === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -,foxxtrot || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:14] awwww [14:15] jtv: what? [14:15] oh, ok [14:37] bryceh: thanks for going the extra km in cleaning up the branch... this is to let you know that I'm approving and you should still be able to land [14:39] henninge: my branch ok now? [14:39] jtv: most likely ... ;) [14:39] the suspense is killing me! [14:42] and [14:42] the [14:42] winner [14:42] is [14:43] ... [14:43] jtv! === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -,jtv || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:43] jtv: r=me [14:43] thanks, looks very good ;-) [14:43] but now that you changed actual code, you will have to run it through ec2 :-P [14:44] henninge: which jtv branch did you review? [14:44] jtv: oh, not the one he asked about today. [14:45] bac: ^ [14:45] this is three days old. [14:45] https://code.edge.launchpad.net/~jtv/launchpad/recife-limited-setcurrenttranslation/+merge/28715 [14:45] oh, ok. henninge just trying not to duplicate work [14:49] henninge: thank you for your kind approval... I'm out of here! [14:50] bac: I'll keep an eye out for any questions you may have, but be aware that Holland is about to face Brazil and I may be distracted or sobbing uncontrollably. :) === matsubara is now known as matsubara-afk [14:50] jtv: good luck. you'll need it. [14:51] indeed! [14:51] jtv: can you keep us posted here? [14:51] * bac is surprised there has not been more WC chatter [14:51] bac: ok... my first time as a commentator [14:52] i watched the ger-eng game on univision. having spanish play-by-play was much less annoying [14:52] gooooooooooooooooooooooooooool alemania! [14:52] YEah! [14:52] ;-) [14:52] * jtv relocates to TV [14:52] jtv, danilos: Err, the potemplate api branch was never approved. [14:53] https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525371/+merge/25423 [14:58] henninge, isn't there a resubmitted MP which jtv approved? [14:58] ? [14:58] danilos: I hope there is [14:58] (wc commentary: Brazilian players looked nervous and tense... is their national anthem particularly difficult like the Argentinian one?) [14:59] adiroiban: Hi! [14:59] (the Dutch players just unfolded a large banner "say no to racism"—a superfluous statement given the diverse composition of the team) [15:00] henninge: hi [15:00] adiroiban: which is the MP that jtv approved for the POTemplate API? [15:01] because this one isn't: [15:01] https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525371/+merge/25423 [15:01] henninge: Danilo was the one doing the final review [15:02] adiroiban: hm, it was landed as [r=jtv], though. [15:02] I did approve that one, didn't I? [15:03] bac: so far the pressure went straight onto the brazilians [15:03] jtv: i don't know what that means. [15:03] ah... yep. Danilo did approve it first... but then we decide that it requires more work and the jtv was the one to appove and land it [15:03] jtv: make lint on your branch shows a lot of stuff...mostly valid i think [15:03] assigned but unused variables [15:03] bac: it means the Dutch have had the upper hand so far. [15:04] jtv: goals only, please. :) [15:05] bac: goals only? The violence may be more interesting... they've deliberately taken down 3 dutch players in 5 minutes and they seem to know the referee will let it go. [15:05] adiroiban: only the MP does not show that. Wonder why. But if nobody objects to you story ... ;-) [15:05] bac: those variables were in there before... they're not mine but they serve an explanatory role. [15:06] henninge: I guess that jtv forget to update the MP [15:06] adiroiban: yes he does forget things once in a while ... ;-) [15:06] adiroiban: I guess... I may have told you by IRC [15:08] jtv: lucky sods [15:09] henninge: they wouldn't have gotten anywhere so far without unpunished fouls... hope the game doesn't turn bilaterally ugly [15:09] * henninge just looked up sods ... probably not the term I meant to choose. [15:10] bac: Brazil just scored [15:10] awwwww [15:11] jtv: come on! We have a date in the final! [15:11] I hope so... [15:12] jtv: imagine a pick up match between you and stevea vs. ursula and salgado. you know how i'd bet... [15:15] bac: are you okay with those explanatory variables or do you want a cleanup round? [15:17] jtv: it's up to you. i don't think they add much by way of explanation. [15:17] bac: hell, I don't want the noise... I'll take care of it. [15:18] bac: the match may not be worth following with the referee already having made up his mind [15:18] open, nonfunctional violence by the brazilians now... no yellow card [15:19] hi bac, can you do a small branch please? [15:19] bigjools: sure, next. [15:19] oh abel is free [15:19] I think? [15:19] I don't mind who does it :) [15:20] * bigjools files MP [15:21] jtv: the comment and code don't seem to match: [15:21] # Strip the last column off the result; it's just always True. [15:21] 146 + return set(result) [15:21] s/146 +// [15:21] bac: yikes! that comment is from an older iteration... I'll take it out [15:21] thx [15:30] jtv: r=me [15:31] bac: great, thanks! [15:34] bigjools: r=me [15:34] bac: easiest soyuz branch evar, thanks ) === bac changed the topic of #launchpad-reviews to: bigOn call: adeuring, bac || reviewing: -,jelmer || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:34] bigjools: that's the truth! === ctrlsoft is now known as jelmer_ [15:37] bac_: Hi! [15:37] :-) [15:37] jelmer_: how are we supposed to find you with a new irc nick every day? :) [15:37] sorry :-) [15:38] jelmer_: looking at your MP, i wonder about the import for Dsc [15:39] jelmer_: on our machines where this script runs do we really find Dsc in both places? [15:39] bac: Ah, I forgot to document that one. [15:39] bac: Newer versions of python-debian have renamed the upstream package from debian_bundle to debian [15:39] it still provides debian_bundle but will raise a DeprecationWarning [15:40] that warning causes various tests to break [15:40] jelmer_: and our machines are in flux between the two? [15:40] or is it a dev vs production issue? [15:40] bac: Maverick has the newer version of python-debian, and we'll hopefully include a more up to date version in the PPA as well soon. [15:42] jelmer_: ok, but eventually we'll all find it in one place. how about an XXX comment for cleaning up the import when things stabilize? [15:43] bac: That's a good idea. === salgado is now known as salgado-lunch [15:45] thanks jelmer_ . r=bac [15:45] bac: Thanks! [15:55] hey bigjools it'd be good to tell your reviewer if you're looking for an RC. it really shouldn't matter but i would look at things much harder if i knew it was an rc candidate. [15:55] otoh, i should've asked... [15:55] well, I don't understand how that affects the review [15:56] bigjools: ideally it shouldn't. [15:56] they should all be super stringent [15:56] but I should have said - in fact I normally do [15:56] so sorry 'bout dat [15:56] but in reality i know i'd pay a lot more attention [16:10] bac: goooooooooooal! === bac changed the topic of #launchpad-reviews to: bigOn call: adeuring, bac || reviewing: -,bjornt || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:27] bac: gooooooooooooooal! [16:27] NED - BRA 2:1 [16:28] jtv is probably dancing Samba ... ;-) [16:28] henninge: good thing I'm not on Mumble [16:28] yeah, it'd probably burst my earphones [16:30] My God, that was a deserved red [16:30] I say [16:30] Stabbing your cleats right into the back of a prone man's leg... [16:33] Damn, that red card should've gone to Caca [16:34] jtv: that was yellow [16:34] yes, but what Caca did deserved red. [16:35] jtv: did you realize that we will be watching the final together in Prague? [16:35] henninge: I'm being very careful in my use of my country's very rich vocabulary for your countrymen ;-) [16:35] Would be fun to watch de-nl together! [16:36] What I expect! ;) [16:37] They need higher goals for those kind of shots ... [16:38] We got lucky there... [16:40] ...and not there [16:50] jtv: Congratulations! [16:50] henninge: thanks! Now you do the same and we'll meet in Prague! [16:51] jtv: We'll meet anyway ... :-P [16:52] :D === salgado-lunch is now known as salgado [16:56] henninge: Sneijder just explained why he kept shooting into the goalie's arms: he knew the guy isn't so good at catching close to his body. [16:57] The surprising thing is I don't hear any vuvuzelas outside... Excellent noise isolation my dad has. [16:58] bac: Holland passes 2—1, deservedly. Loyalty aside I was sort of rooting for Brazil until the first minutes when the fouls started. [16:59] jtv: nice [17:02] jtv: enjoy your weekend! I'll be going there now, too. ;-) [17:02] henninge: thanks, you too... have a lemonade on us! [17:02] I will ! ;-) [17:12] jtv, excellent, thank you! [17:12] bryceh: whoa, took me a while to figure out what that was about. :) === adeuring changed the topic of #launchpad-reviews to: On call: bac || reviewing: bjornt || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [17:51] bac: apparently the Brazilian fans were good sports and many said the Dutch team was simply better in the second half. I think the referee let a few Dutch fouls slip later on to compensate for some really bad ones he let go from the other side earlier. [17:53] I wish we'd get instant replay soon... bad calls breed lots of resentment on both sides that they take out on each other. === matsubara-afk is now known as matsubara === sinzui changed the topic of #launchpad-reviews to: On call: bac || reviewing: bjornt || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [18:26] Hi bac. I have a branch for review. My apologies for the diff which I think is hard to read because of the many small changes needed to expose summary and copyright for packages. [18:29] sinzui: ok === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: sinzui || queue: [bjornt] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mwhudson_ is now known as mwhudson [18:44] sinzui: i get errors in test_highlighted_copyright_is_None [18:44] :( [18:46] I get them too [18:46] one moment [18:47] ah [18:49] bac: The setup is wrong. The copyright must be set to None. Our factory is better than our data, or some of our uploaders [18:50] bac: this is the fix for when there is no copyright file: http://pastebin.ubuntu.com/458417/ [19:11] sinzui: if you go to https://launchpad.dev/ubuntu/warty/+source/a52dec you'll see a warning that there is no current release [19:11] yes, that is true [19:11] clicking on the +changelog gives a nice but empty page [19:11] +copyright oopses [19:11] bac: sample data is very very bad [19:11] remember that we have to use the factory to make SPPH for everything, that page is ehy [19:12] so if it were fully fleshed out the oops would not happen [19:12] pmount and mozilla-firefox have some data [19:13] sinzui: ok. i just want assurance that we're being suffiently defensive [19:13] The page told the truth about no release, and we do not make links to impossible packages [19:13] however, I will make copyright not oops when someone is being bad [19:17] bac: this is the fix to see info that there is no copyright: http://pastebin.ubuntu.com/458422/ [19:17] great [19:27] sinzui, ping, when you have a moment, could you please review https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116 ? [19:28] mars: I do not think that will work [19:28] neither do I [19:28] '|' is not OR, is it [19:28] mars tales is testing None or something [19:29] False is something [19:29] eewww, even worse [19:29] * sinzui thinks if there is a tales formatter for false>None [19:29] surely there is some way to just do a binary expression in TALES? [19:30] no [19:30] but you can do: [19:30] python: is_edge or is_lpnet [19:30] do the variables come through? [19:30] from TALES back to Python ? [19:31] They do in this case I believe. We scoped them to the entire template [19:31] simple, testing now [19:31] mars, if not you need to do python: config.launchpad.is_edge ?? [19:33] sinzui, python: is_edge or is_lpnet works [19:34] rock [19:36] branch scanner is slow today [19:36] pushed, just waiting for the update [19:38] updated [19:40] sinzui, r=you? [19:40] mars: yes r=me [19:41] I wonder if 'ec2 land' understands the release-critical tag [19:41] thanks sinzui [19:42] mars: it adds [release-critical=] if there's a review with that tag iirc === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [bjornt] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [19:42] mwhudson: that is correct [19:42] cool [19:45] sinzui, could you please hit 'Approve' on the merge proposal? https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116 === 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 [19:48] bac, care to give a second a review of https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116 ? [19:48] sure [19:49] mars i'm confused [19:49] you reviewed your own branch? [19:49] bac, I don't know how to formally say 'Curtis reviewed it' - there is no Proxy Review button [19:50] mars: why didn't he mark the MP? [19:50] I can vote by proxy - why not review by proxy as well? [19:50] the commit message is going to be a hoot [19:50] :D [19:50] [r=mars][rc=mars][bug=mars] [19:51] bac, I don't know, he isn't answering [19:51] i would actually suggest you get an rc from curtis since he was the last RM. i'd be very uncomfortable giving myself an rc [19:51] bac, I can do the r-c myself [19:51] you certainly can. i think it is a bad idea. that's just me. [19:51] you know, by being Release Manager and all [19:52] true, but that is why I asked for your second code review [19:52] from a RM perspective, this has to land to fix the lp_lucid builder [19:52] so there is no question around the RC [19:53] bac, ok, Curtis reviewed it - could you still take a look at the code, just to make sure it's sane? [19:54] mars: doing so now [19:54] thanks [19:56] mars, what about staging and dogfood? [19:56] bac, no, we do not want to track the usage of those servers. They will just pollute the data. [19:56] Since they are test environments [19:57] mars: fair enough. r=me [19:57] thanks bac === rinze is now known as jelmer_ === mwhudson_ is now known as mwhudson === matsubara is now known as matsubara-afk