=== henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue: [wgrant,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [08:34] wgrant: Hi! I only see one unreviewed propposal from you. [08:34] henninge: There's one which has two DB reviews but needs a code review, and one which is entirely unreviewed. [08:35] Actually, there are two of the former class, but the older one should be disregarded. [08:35] * wgrant finds a link. [08:35] wgrant: a, I just saw all the green lights [08:35] ;-) [08:35] https://code.edge.launchpad.net/~wgrant/launchpad/bprdc === henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [10:01] wgrant: r=me [10:01] on both accounts === henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [10:01] henninge: Thanks. [10:02] bigjools: No final objections to https://code.edge.launchpad.net/~wgrant/launchpad/bprdc or https://code.edge.launchpad.net/~wgrant/launchpad/get-out-of-here-p-a-s? [10:09] wgrant: just looking [10:16] wgrant: I don't think pas.py belongs in model, but there's nowhere better either :/ [10:16] bigjools: That was my thinking. [10:16] there's a smattering of stuff in adapters [10:16] Right, I looked there. [10:16] But it's barely more relevant,. [10:16] indeed [10:16] It is sort of model. [10:16] Just not DB model. [10:17] model means db model reallky [10:17] Shhh. [10:18] it could go under /soyuz [10:18] In the root of the app package, like it is in buildmaster? [10:19] yep [10:19] Hm, some other apps do do that. [10:19] Just not Soyuz. [10:19] Good idea. [10:19] that was my thinking [10:23] bigjools: Move pushing. [10:28] henninge: Can you please land those two? [10:29] wgrant: has the prerequisite already filtered through to db-devel? [10:29] henninge: It landed Fridayish... let me see. [10:29] (landed on devel, that is) [10:30] henninge: Yeah, it's in db-devel. [10:30] wgrant: cool, I'll land them [10:30] Thanks. === intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [10:32] wgrant: having said that, can you please merge the current devel / db-devel respectively and push? I like to do that before landing. ;-) === jtv1 is now known as jtv [10:38] henninge: Done. [10:38] wgrant: cheers [10:59] henninge: hi, Do you have time to review this branch https://code.edge.launchpad.net/~adiroiban/launchpad/bug-359180-take-2/+merge/20122? [10:59] I emailed Graham, and he told me he will not have time do review it and told me to ask the OCR [11:00] adiroiban: put it on the queue, I or intellectronica will pick it up. [11:00] s/on/in/ === adiroiban changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, - || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [11:18] henninge: Those are ec2ing happily? [11:19] wgrant: sorry, got distracted. I'll start them right now. [11:19] henninge: Thanks. [11:24] wgrant: the "ec2" command is failing for me "ImportError: No module named _pythonpath" [11:24] any idea [11:25] henninge: The branch is unbuilt? [11:25] wgrant: I thought it wasn't ... [11:25] * henninge builds [11:30] wgrant: ok, first one's on its way. [11:41] wgrant: second one, too. Have a good night! === henninge changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: lunch, - || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [11:42] henninge: Thanks! Have a nice day. === matsubara-afk is now known as matsubara === daniloff is now known as danilos === mrevell is now known as mrevell-lunch === jamalta-afk is now known as jamalta [13:34] jml: Do you have some time this afternoon to finalise a review that Bjorn was doing last week? It's the isolate-tests branch that adapts subunit's IsolatedTestCase to work with zope.testing. I think it might just need a final nod; Bjorn and I went back-and-forth about it last week. [13:34] https://code.edge.launchpad.net/~allenap/launchpad/isolate-tests/+merge/20916 [13:34] allenap, I can't promise anything, but I'll definitely try. [13:36] jml: Thank you! The comment where Bjorn abstains is probably the place to start if you're happy to skip the earlier conversation. [13:37] allenap, ta [13:45] danilos: hi, Can I add you as a reviewer for this MP https://code.edge.launchpad.net/~adiroiban/launchpad/bug-201749/+merge/21250 or should I as the OCR? [13:46] adiroiban, I'd like to review it, so yes :) [13:46] adiroiban, well, someone from translations should, you can OCR it if henninge is around, that's even better :) [13:50] intellectronica, hi. Could I get you to review a bug heat flames branch for me? [13:52] danilos: ok. I will see if Henning has time to do it, otherwise I will assign the MP to you. === henninge_ changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, - || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === henninge_ is now known as henninge [13:53] Have you received the email for POTemplate details API pre-implementation review? [13:53] adiroiban, thanks, and yes, I've received it, and will look into it [13:54] adiroiban: I will look at it now. === henninge changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adiroiban, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [13:54] henninge: ok. in the queue there was another MP [13:55] adiroiban: oh, I did not see that. [13:55] adiroiban: which one should I do first, then? [13:55] the MP for bug 201749 needs to be reviewed by someone from the Rosetta team [13:55] Bug #201749: Impossible to mark as needing review strings already translated or with suggestions [13:55] henninge: this one https://code.edge.launchpad.net/~adiroiban/launchpad/bug-201749/+merge/21250 [13:55] :) === henninge changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adiroiban, - || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [13:57] gmb: Hi, Any news about the landing of this MP https://code.edge.launchpad.net/~adiroiban/launchpad/bug-512307/+merge/20442 ? === mrevell-lunch is now known as mrevell [14:10] adiroiban: I think it vanised; I saw no emails about it and the EC2 instance shut down cleanly. I'll re-run it but this probably means there's a test failure somewhere. [14:14] gmb: yes, there must be a failing tests on ec2. I have ran a full translation test suite on my computer and there were no errors. [14:21] adiroiban: That's discouraging. Well, it's running now and I'm capturing the output, so hopefully we'll get some answers. [14:23] intellectronica: Are you up for reviewing a branch that was once reviewed and approved by henninge but then underwent some big changes? https://code.edge.launchpad.net/~allenap/launchpad/twisted-threading-bug-491870/+merge/21376 [14:26] gmb, there were problems with ec2land some ~7 days ago or so, it required updating your download-cache and tree for the ec2 land to go headless and succeed :) [14:26] danilos: This isn't an ec2 land problem, it's an ec2 test run that's just dying a death. Another run started at the same time succeed, however. [14:27] allenap: sure [14:28] intellectronica: Thank you :) === intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: adiroiban, allenap || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adiroiban, allenap, - || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [14:55] allenap: fyi i have a ui call, now, so will only start reviewing your branch in ~1h. hope that's ok. [14:55] intellectronica: That's great. [15:00] intellectronica, I'll add mine to the queue, too. https://code.edge.launchpad.net/~deryck/launchpad/bug-page-distro-max-heat-532099/+merge/21370 === deryck changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adiroiban, allenap, - || queue: [adiroiban(http://tinyurl.com/yex2mcx),deryck] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-lunch [15:19] abentley, can you review https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-apidoc/+merge/21181 ? [15:19] i want gary to look at it, but he's busy. if you can do the review i can just get a quick ok from him afterwards [15:20] leonardr, sure. === abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adiroiban, allenap, leonardr || queue: [adiroiban(http://tinyurl.com/yex2mcx),deryck] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [15:48] leonardr, why are you combining create-lp-wadl.py and apiindex.py ? [15:49] abentley: it's simpler for a number of reasons [15:49] for instance, generating index.html requires an ordered list of the web service versions [15:49] which is only available when launchpad is running === salgado is now known as salgado-lunch [15:50] also, changing the makefile to call apiindex.py many times made it very messy [15:51] leonardr, why are you printing out the raw HTML in apidoc.txt, rather than extracting the text? [15:52] abentley: no reason. if you tell me how to extract the text i'll change it [15:53] leonardr, you call "extract_text(html)" [15:54] ok, changing it now [15:54] thanks [15:54] leonardr, no problem. [15:55] leonardr, you asked if there was a way to put apidoc-index.pt somewhere else. I think you could do that by subclassing PageTemplate rather than using PageTemplateFile. [15:59] abentley, i don't see any examples in launchpad. do you know how to use it off the top of your head or is this something i should bring to gary? [16:00] * leonardr is looking online now [16:02] leonardr, that's an educated guess. [16:02] abentley: ok, i'll keep looking and maybe gary can help, that's the kind of thing i wanted to ask him [16:02] It should be possible. The question in my mind is effort to benefit. [16:03] But Il'll look later and see if a quick way presents itself to me [16:03] gary: thanks === deryck is now known as deryck[lunch] === jamalta is now known as jamalta-afk [16:04] allenap: r=me === intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adiroiban, deryck, leonardr || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [16:05] intellectronica: Woohoo, thanks :) === jamalta-afk is now known as jamalta === matsubara-lunch is now known as matsubara === bac` is now known as bac === intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adiroiban, -, leonardr || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [16:27] leonardr, is "last_version_with_mutator_named_operations" meant to be authoritative? [16:28] abentley: yes, in the sense that the value of that setting controls the behavior of lazr.restful [16:28] leonardr, i.e. is there a new policy against having mutators? [16:28] abentley: there's a policy against publishing named operations that do exactly the same thing as a mutator [16:29] leonardr, where can I find that policy? [16:30] abentley: it's something we did by fiat, we didn't think it would cause problems. if you have concerns we can discuss it before i land the branch [16:32] Okay, so by "exactly the same thing as a mutator", do you mean "exactly the same thing as an existing mutator does", or do you mean "exactly the same thing as a mutator would do, whether or not it exists"? [16:32] abentley: i mean that if you publish a named operation as a mutator, it is _only_ available as a mutator, not as a mutator + a named operation [16:33] leonardr, I see. That sounds reasonable. [16:33] ok, great [16:36] leonardr, You removed the steps in lib/lp/bugs/stories/webservice/xx-bug.txt that set the bug to New/Low. Have you verified that the tests which follow are still testing the right thing? [16:39] abentley: i think you're reviewing the precursor branch [16:39] which is fine, but it's already been reviewed. i put up a diff against the precursor branch in a comment [16:39] btw, yes, i have run that branch through ec2 successfully [16:40] leonardr, I didn't see a huge difference between the diffs, except that the one for the merge proposal is colourized. [16:41] abentley: hmm [16:41] maybe i pasted a bad diff [16:42] abentley: try http://paste.ubuntu.com/395699/ [16:42] leonardr, I wasn't asking whether the tests pass, I was asking whether the tests were still testing the right thing. For example, if a subsequent test had set the status to "fix committed", and then checked whether it was "fix committed", it would no longer be a good test. [16:43] Because the starting value is already fix committed. [16:43] abentely: i'll double check, but i'm pretty sure they're testing the right thing [16:44] leonardr, anyhow, I accept that they're out of scope. [16:45] for this review, I mean. [16:45] abentley: i wasn't asked that in the last review so i think it's fair to double check [16:46] leonardr, when you pastebin a diff, it's nice if you select its syntax as "Diff", so that it gets colourized like this: http://paste.ubuntu.com/395702/ [16:47] abentley: ok, will do in the future [16:48] leonardr, so aside from the extract_text thing, this looks fine. [16:49] abentley: ok, do you have any suggestions for fixing the ZTL to do a lookup like config/active_versions/$version? [16:50] leonardr, I don't follow. [16:50] abentley: right now i have python code that creates a list of tuples versions_and_descriptions [16:50] in the ZTL i loop over that list to do something for every version [16:51] if there was some way in ZTL to look up the description for a given version, i wouldn't have to create that list of tuples [16:51] but while i can navigate a dictionary from ZTL, I have to know the dictionary key ahead of time [16:51] so i can do config/active_versions/beta, but not config/active_versions/$version [16:51] since i don't know much about ZTL i wonder if there's something i'm missing === salgado-lunch is now known as salgado [16:55] abentley: anyway, that's another thing gary can weigh in on [16:55] leonardr, if you can access config, you should be able to loop over config/active_versions in the ZTL, and access the descriptions as config/version_descriptions/key [16:56] abentley: that won't try to access version_descriptions['key']? [16:56] leonardr, I don't think so. [16:56] ok, i'll try it [16:57] leonardr, I am assuming 'key' is the repeat variable, of course. [16:57] right === gary_poster is now known as gary-lunch === deryck[lunch] is now known as deryck [17:09] abentley: unfortunately that doesn't work [17:09] KeyError: 'version' [17:10] "version" is your repeat variable? [17:10] abentley: yes [17:10] see http://paste.ubuntu.com/395709/ [17:11] i know i can do it with a python expression but i also know we're not supposed to do use those [17:13] leonardr, maybe that needs to be config/version_descriptions/${version} === danilos is now known as daniloff [17:19] abentley: KeyError: '${version}' [17:19] leonardr, sorry I guess I gave bad advice. Quite surprised that's a problem. [17:20] abentley: np === salgado changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: adiroiban, -, leonardr || queue: [adiroiban(http://tinyurl.com/yex2mcx),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === henninge changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: -, leonardr || queue: [adiroiban(http://tinyurl.com/yex2mcx),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === gary-lunch is now known as gary_poster === jtv is now known as jtv-zzz === intellectronica changed the topic of #launchpad-reviews to: on call: abentley || reviewing: -, leonardr || queue: [adiroiban(http://tinyurl.com/yex2mcx),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: leonardr || queue: [adiroiban(http://tinyurl.com/yex2mcx),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-afk === NCommander is now known as Guest5889 === abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: salgado || queue: [adiroiban(http://tinyurl.com/yex2mcx),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews === salgado is now known as salgado-afk === james_w changed the topic of #launchpad-reviews to: on call: abentley || reviewing: salgado || queue: [adiroiban(http://tinyurl.com/yex2mcx),salgado,james_w(http://is.gd/aIvTS)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [20:36] A small branch that will make several people happy just added to the queue: https://code.edge.launchpad.net/~james-w/launchpad/merge-proposal-instructions/+merge/21401 [20:36] abentley: if you can squeeze it in before EOD ^ [20:41] james_w, could you have the test expand out a bit more than just the 'bzr merge ...' Maybe 'bzr merge bzr+ssh://~person-name.../fooix/...' or something? [20:42] rockstar: doesn't the other test cover that? [20:44] james_w, I guess it does. It uses sample data though, which I think is generally icky. [20:44] it's just echoing what the rest of the test is doing [20:44] james_w, okay. r=me [20:45] thanks [20:45] would you submit for me please? [20:48] james_w, okay. [20:48] abentley, is your queue really reflective of reality? [20:54] rockstar, no, not really. === abentley changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [adiroiban(http://tinyurl.com/yex2mcx)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [20:56] james_w, sorry, was on the phone. [20:57] abentley, could you possibly do a quick review for me? [20:58] (I know we're approaching EOD for you, feel free to tell me "no") [20:58] rockstar, I'll look at it. [20:58] https://code.edge.launchpad.net/~rockstar/launchpad/branch-upgrade-copy-bzr/+merge/21399 [21:00] rockstar, what happened with db_branch_transport? [21:01] abentley, artifact of me messing with the test. :/ Removed. [21:02] rockstar, cool. Actual changes look fine. What's the problem with testing? [21:02] abentley, well, I'm still not entirely sure why the existing test didn't fail in the new way... :/ [21:03] i.e. it demonstrated that it fixed the old backup.bzr bug, but it really should have demonstrated the new one. [21:03] As it stands now, it looks like I just refactored a bit. [21:06] rockstar, I think it's because your test isn't authentic enough; remember, you can create a new directory on top of an empty one, and your backup.bzr is an empty directory. [21:08] abentley, okay, so maybe I should copy .bzr to backup.bzr - How's that sound? [21:08] rockstar, sounds good to me. [21:11] abentley, so, that seems to give me the error: ReadError: Error reading from '~person-name9/product-name4/branch11/.bzr' [21:11] rockstar, copying does that? [21:12] Even though I can do source_branch_transport.list_dir('.') and it results in ['.bzr'] [21:12] Yea, no idea what's going on. [21:12] source_branch_transport.copy('.bzr', 'backup.bzr') [21:13] rockstar, I'm not sure that copy is meant to recursively copy trees. In fact, experience suggests that it's not meant for that. [21:13] abentley, yeah, that's what I was just thinking. [21:13] rockstar, perhaps you could try copy_tree_to_transport again? [21:13] abentley, yeah, lemme do that. [21:17] abentley, hooray! It actually found the bug I really needed to squish. [21:18] abentley, here's the diff I'm committing: http://pastebin.ubuntu.com/395817/ [21:20] rockstar, lines 9-10 are "look before you leap". Have you considered doing "easier to ask forgiveness than permission"? [21:20] abentley, I could wrap it in try/except. If I don't check, a test will fail (currently) [21:21] I'm not sure try/except is really better than if in this case. [21:21] rockstar, yes, EAFP is often implemented via try/except blocks. [21:22] rockstar, it is a better habit to be into. [21:22] abentley, so catch the exception and pass? [21:22] rockstar, right. [21:23] abentley, okay. I've generally avoided just passing, but I'm okay with it if you are. [21:23] rockstar, as long as you're handling a specific error, that's fine with me. [21:23] abentley, cool. [21:24] abentley, changes pushed. [21:27] ...and the diff shows the changes. [21:31] rockstar, r=me [21:31] abentley, cheers sir, and thanks for the input. [21:31] rockstar, no problem. === matsubara-afk is now known as matsubara === jamalta is now known as jamalta-afk [22:18] anyone want to review a branch? [22:18] it's pretty small [22:21] https://code.edge.launchpad.net/~mwhudson/launchpad/back-off-failing-imports-bug-413637/+merge/21408 === adiroiban changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [adiroiban(http://tinyurl.com/yex2mcx),adiroiban(bug-522188-take-2)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews