[08:34] <henninge> wgrant: Hi! I only see one unreviewed propposal from you.
[08:34] <wgrant> henninge: There's one which has two DB reviews but needs a code review, and one which is entirely unreviewed.
[08:35] <wgrant> Actually, there are two of the former class, but the older one should be disregarded.
[08:35]  * wgrant finds a link.
[08:35] <henninge> wgrant: a, I just saw all the green lights
[08:35] <henninge> ;-)
[08:35] <wgrant> https://code.edge.launchpad.net/~wgrant/launchpad/bprdc
[10:01] <henninge> wgrant: r=me
[10:01] <henninge> on both accounts
[10:01] <wgrant> henninge: Thanks.
[10:02] <wgrant> 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] <bigjools> wgrant: just looking
[10:16] <bigjools> wgrant: I don't think pas.py belongs in model, but there's nowhere better either :/
[10:16] <wgrant> bigjools: That was my thinking.
[10:16] <bigjools> there's a smattering of stuff in adapters
[10:16] <wgrant> Right, I looked there.
[10:16] <wgrant> But it's barely more relevant,.
[10:16] <bigjools> indeed
[10:16] <wgrant> It is sort of model.
[10:16] <wgrant> Just not DB model.
[10:17] <bigjools> model means db model reallky
[10:17] <wgrant> Shhh.
[10:18] <bigjools> it could go under /soyuz
[10:18] <wgrant> In the root of the app package, like it is in buildmaster?
[10:19] <bigjools> yep
[10:19] <wgrant> Hm, some other apps do do that.
[10:19] <wgrant> Just not Soyuz.
[10:19] <wgrant> Good idea.
[10:19] <bigjools> that was my thinking
[10:23] <wgrant> bigjools: Move pushing.
[10:28] <wgrant> henninge: Can you please land those two?
[10:29] <henninge> wgrant: has the prerequisite already filtered through to db-devel?
[10:29] <wgrant> henninge: It landed Fridayish... let me see.
[10:29] <wgrant> (landed on devel, that is)
[10:30] <wgrant> henninge: Yeah, it's in db-devel.
[10:30] <henninge> wgrant: cool, I'll land them
[10:30] <wgrant> Thanks.
[10:32] <henninge> wgrant: having said that, can you please merge the current devel / db-devel respectively and push? I like to do that before landing. ;-)
[10:38] <wgrant> henninge: Done.
[10:38] <henninge> wgrant: cheers
[10:59] <adiroiban> 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] <adiroiban> I emailed Graham, and he told me he will not have time do review it and told me to ask the OCR
[11:00] <henninge> adiroiban: put it on the queue, I or intellectronica will pick it up.
[11:00] <henninge> s/on/in/
[11:18] <wgrant> henninge: Those are ec2ing happily?
[11:19] <henninge> wgrant: sorry, got distracted. I'll start them right now.
[11:19] <wgrant> henninge: Thanks.
[11:24] <henninge> wgrant: the "ec2" command is failing for me "ImportError: No module named _pythonpath"
[11:24] <henninge> any idea
[11:25] <wgrant> henninge: The branch is unbuilt?
[11:25] <henninge> wgrant: I thought it wasn't ...
[11:25]  * henninge builds
[11:30] <henninge> wgrant: ok, first one's on its way.
[11:41] <henninge> wgrant: second one, too. Have a good night!
[11:42] <wgrant> henninge: Thanks! Have a nice day.
[13:34] <allenap> 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] <allenap> https://code.edge.launchpad.net/~allenap/launchpad/isolate-tests/+merge/20916
[13:34] <jml> allenap, I can't promise anything, but I'll definitely try.
[13:36] <allenap> 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] <jml> allenap, ta
[13:45] <adiroiban> 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] <danilos> adiroiban, I'd like to review it, so yes :)
[13:46] <danilos> adiroiban, well, someone from translations should, you can OCR it if henninge is around, that's even better :)
[13:50] <deryck> intellectronica, hi.  Could I get you to review a bug heat flames branch for me?
[13:52] <adiroiban> danilos: ok. I will see if Henning has time to do it, otherwise I will assign the MP to you.
[13:53] <adiroiban> Have you received the email for POTemplate details API pre-implementation review?
[13:53] <danilos> adiroiban, thanks, and yes, I've received it, and will look into it
[13:54] <henninge> adiroiban: I will look at it now.
[13:54] <adiroiban> henninge: ok. in the queue there was another MP
[13:55] <henninge> adiroiban: oh, I did not see that.
[13:55] <henninge> adiroiban: which one should I do first, then?
[13:55] <adiroiban> the MP for bug 201749 needs to be reviewed by someone from the Rosetta team
[13:55] <mup> Bug #201749: Impossible to mark as needing review strings already translated or with suggestions <story-translations-review> <ui> <Launchpad Translations:In Progress by adiroiban> <https://launchpad.net/bugs/201749>
[13:55] <adiroiban> henninge: this one https://code.edge.launchpad.net/~adiroiban/launchpad/bug-201749/+merge/21250
[13:55] <adiroiban> :)
[13:57] <adiroiban> gmb: Hi, Any news about the landing of this MP https://code.edge.launchpad.net/~adiroiban/launchpad/bug-512307/+merge/20442 ?
[14:10] <gmb> 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] <adiroiban> 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] <gmb> adiroiban: That's discouraging. Well, it's running now and I'm capturing the output, so hopefully we'll get some answers.
[14:23] <allenap> 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] <danilos> 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] <gmb> 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] <intellectronica> allenap: sure
[14:28] <allenap> intellectronica: Thank you :)
[14:55] <intellectronica> allenap: fyi i have a ui call, now, so will only start reviewing your branch in ~1h. hope that's ok.
[14:55] <allenap> intellectronica: That's great.
[15:00] <deryck> intellectronica, I'll add mine to the queue, too.  https://code.edge.launchpad.net/~deryck/launchpad/bug-page-distro-max-heat-532099/+merge/21370
[15:19] <leonardr> abentley, can you review https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-apidoc/+merge/21181 ?
[15:19] <leonardr> 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] <abentley> leonardr, sure.
[15:48] <abentley> leonardr, why are you combining create-lp-wadl.py and apiindex.py ?
[15:49] <leonardr> abentley: it's simpler for a number of reasons
[15:49] <leonardr> for instance, generating index.html requires an ordered list of the web service versions
[15:49] <leonardr> which is only available when launchpad is running
[15:50] <leonardr> also, changing the makefile to call apiindex.py many times made it very messy
[15:51] <abentley> leonardr, why are you printing out the raw HTML in apidoc.txt, rather than extracting the text?
[15:52] <leonardr> abentley: no reason. if you tell me how to extract the text i'll change it
[15:53] <abentley> leonardr, you call "extract_text(html)"
[15:54] <leonardr> ok, changing it now
[15:54] <leonardr> thanks
[15:54] <abentley> leonardr, no problem.
[15:55] <abentley> 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] <leonardr> 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] <abentley> leonardr, that's an educated guess.
[16:02] <leonardr> abentley: ok, i'll keep looking and maybe gary can help, that's the kind of thing i wanted to ask him
[16:02] <gary_poster> It should be possible.  The question in my mind is effort to benefit.
[16:03] <gary_poster> But Il'll look later and see if a quick way presents itself to me
[16:03] <leonardr> gary: thanks
[16:04] <intellectronica> allenap: r=me
[16:05] <allenap> intellectronica: Woohoo, thanks :)
[16:27] <abentley> leonardr, is "last_version_with_mutator_named_operations" meant to be authoritative?
[16:28] <leonardr> abentley: yes, in the sense that the value of that setting controls the behavior of lazr.restful
[16:28] <abentley> leonardr, i.e. is there a new policy against having mutators?
[16:28] <leonardr> abentley: there's a policy against publishing named operations that do exactly the same thing as a mutator
[16:29] <abentley> leonardr, where can I find that policy?
[16:30] <leonardr> 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] <abentley> 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] <leonardr> 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] <abentley> leonardr, I see.  That sounds reasonable.
[16:33] <leonardr> ok, great
[16:36] <abentley> 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] <leonardr> abentley: i think you're reviewing the precursor branch
[16:39] <leonardr> which is fine, but it's already been reviewed. i put up a diff against the precursor branch in a comment
[16:39] <leonardr> btw, yes, i have run that branch through ec2 successfully
[16:40] <abentley> leonardr, I didn't see a huge difference between the diffs, except that the one for the merge proposal is colourized.
[16:41] <leonardr> abentley: hmm
[16:41] <leonardr> maybe i pasted a bad diff
[16:42] <leonardr> abentley: try http://paste.ubuntu.com/395699/
[16:42] <abentley> 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] <abentley> Because the starting value is already fix committed.
[16:43] <leonardr> abentely: i'll double check, but i'm pretty sure they're testing the right thing
[16:44] <abentley> leonardr, anyhow, I accept that they're out of scope.
[16:45] <abentley> for this review, I mean.
[16:45] <leonardr> abentley: i wasn't asked that in the last review so i think it's fair to double check
[16:46] <abentley> 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] <leonardr> abentley: ok, will do in the future
[16:48] <abentley> leonardr, so aside from the extract_text thing, this looks fine.
[16:49] <leonardr> abentley: ok, do you have any suggestions for fixing the ZTL to do a lookup like config/active_versions/$version?
[16:50] <abentley> leonardr, I don't follow.
[16:50] <leonardr> abentley: right now i have python code that creates a list of tuples versions_and_descriptions
[16:50] <leonardr> in the ZTL i loop over that list to do something for every version
[16:51] <leonardr> 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] <leonardr> but while i can navigate a dictionary from ZTL, I have to know the dictionary key ahead of time
[16:51] <leonardr> so i can do config/active_versions/beta, but not config/active_versions/$version
[16:51] <leonardr> since i don't know much about ZTL i wonder if there's something i'm missing
[16:55] <leonardr> abentley: anyway, that's another thing gary can weigh in on
[16:55] <abentley> 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] <leonardr> abentley: that won't try to access version_descriptions['key']?
[16:56] <abentley> leonardr, I don't think so.
[16:56] <leonardr> ok, i'll try it
[16:57] <abentley> leonardr, I am assuming 'key' is the repeat variable, of course.
[16:57] <leonardr> right
[17:09] <leonardr> abentley: unfortunately that doesn't work
[17:09] <leonardr> KeyError: 'version'
[17:10] <abentley> "version" is your repeat variable?
[17:10] <leonardr> abentley: yes
[17:10] <leonardr> see http://paste.ubuntu.com/395709/
[17:11] <leonardr> i know i can do it with a python expression but i also know we're not supposed to do use those
[17:13] <abentley> leonardr, maybe that needs to be config/version_descriptions/${version}
[17:19] <leonardr> abentley: KeyError: '${version}'
[17:19] <abentley> leonardr, sorry I guess I gave bad advice.  Quite surprised that's a problem.
[17:20] <leonardr> abentley: np
[20:36] <james_w> 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] <james_w> abentley: if you can squeeze it in before EOD ^
[20:41] <rockstar> 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] <james_w> rockstar: doesn't the other test cover that?
[20:44] <rockstar> james_w, I guess it does.  It uses sample data though, which I think is generally icky.
[20:44] <james_w> it's just echoing what the rest of the test is doing
[20:44] <rockstar> james_w, okay.  r=me
[20:45] <james_w> thanks
[20:45] <james_w> would you submit for me please?
[20:48] <rockstar> james_w, okay.
[20:48] <rockstar> abentley, is your queue really reflective of reality?
[20:54] <abentley> rockstar, no, not really.
[20:56] <abentley> james_w, sorry, was on the phone.
[20:57] <rockstar> abentley, could you possibly do a quick review for me?
[20:58] <rockstar> (I know we're approaching EOD for you, feel free to tell me "no")
[20:58] <abentley> rockstar, I'll look at it.
[20:58] <rockstar> https://code.edge.launchpad.net/~rockstar/launchpad/branch-upgrade-copy-bzr/+merge/21399
[21:00] <abentley> rockstar, what happened with db_branch_transport?
[21:01] <rockstar> abentley, artifact of me messing with the test.  :/  Removed.
[21:02] <abentley> rockstar, cool.  Actual changes look fine.  What's the problem with testing?
[21:02] <rockstar> abentley, well, I'm still not entirely sure why the existing test didn't fail in the new way...  :/
[21:03] <rockstar> i.e. it demonstrated that it fixed the old backup.bzr bug, but it really should have demonstrated the new one.
[21:03] <rockstar> As it stands now, it looks like I just refactored a bit.
[21:06] <abentley> 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] <rockstar> abentley, okay, so maybe I should copy .bzr to backup.bzr - How's that sound?
[21:08] <abentley> rockstar, sounds good to me.
[21:11] <rockstar> abentley, so, that seems to give me the error: ReadError: Error reading from '~person-name9/product-name4/branch11/.bzr'
[21:11] <abentley> rockstar, copying does that?
[21:12] <rockstar> Even though I can do source_branch_transport.list_dir('.') and it results in ['.bzr']
[21:12] <rockstar> Yea, no idea what's going on.
[21:12] <rockstar> source_branch_transport.copy('.bzr', 'backup.bzr')
[21:13] <abentley> 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] <rockstar> abentley, yeah, that's what I was just thinking.
[21:13] <abentley> rockstar, perhaps you could try copy_tree_to_transport again?
[21:13] <rockstar> abentley, yeah, lemme do that.
[21:17] <rockstar> abentley, hooray!  It actually found the bug I really needed to squish.
[21:18] <rockstar> abentley, here's the diff I'm committing: http://pastebin.ubuntu.com/395817/
[21:20] <abentley> rockstar, lines 9-10 are "look before you leap".  Have you considered doing "easier to ask forgiveness than permission"?
[21:20] <rockstar> abentley, I could wrap it in try/except.  If I don't check, a test will fail (currently)
[21:21] <rockstar> I'm not sure try/except is really better than if in this case.
[21:21] <abentley> rockstar, yes, EAFP is often implemented via try/except blocks.
[21:22] <abentley> rockstar, it is a better habit to be into.
[21:22] <rockstar> abentley, so catch the exception and pass?
[21:22] <abentley> rockstar, right.
[21:23] <rockstar> abentley, okay.  I've generally avoided just passing, but I'm okay with it if you are.
[21:23] <abentley> rockstar, as long as you're handling a specific error, that's fine with me.
[21:23] <rockstar> abentley, cool.
[21:24] <rockstar> abentley, changes pushed.
[21:27] <rockstar> ...and the diff shows the changes.
[21:31] <abentley> rockstar, r=me
[21:31] <rockstar> abentley, cheers sir, and thanks for the input.
[21:31] <abentley> rockstar, no problem.
[22:18] <mwhudson> anyone want to review a branch?
[22:18] <mwhudson> it's pretty small
[22:21] <mwhudson> https://code.edge.launchpad.net/~mwhudson/launchpad/back-off-failing-imports-bug-413637/+merge/21408