[12:33] <deryck> mrevell, hi.  Can you do a text review for me?
[12:33] <mrevell> Sure deryck
[12:34] <deryck> mrevell, https://code.edge.launchpad.net/~deryck/launchpad/better-dupe-handling-ui/+merge/30309 (see last comment on MP)
[12:34] <deryck> mrevell, thanks!
[12:34]  * mrevell looks
[12:46] <mrevell> deryck, I meant to tag that as "text"
[12:47] <mrevell> deryck, It's a hard one to get right. I'm not saying mine's perfect, so let's talk about it if you're unhappy.
[12:47] <deryck> mrevell, ok, thanks, looking now...
[12:48] <mrevell> Heh, just re-read it and noticed that I didn't finish typng my comment
[12:49] <mrevell> deryck, Sorry, please refresh.
[12:50] <deryck> ok, refreshing now.  sorry too many xchat tabs today.
[12:51] <deryck> mrevell, yeah, I like that better.  I'll fix and submit a new screenshot for you now.
[12:58] <benji> can someone take a look at https://code.edge.launchpad.net/~benji/launchpad/bug-597324/+merge/30330?  bzr lp-land refuses to operate on it with the error "bzr: ERROR: bzrlib.plugins.pqm.lpland.MissingReviewError: Need approved votes in order to land."
[12:59] <deryck> mrevell, new screenie up on the MP.
[12:59]  * mrevell looks
[12:59] <benji> It's marked as "Approved" but the only vote was "Needs Fixing"
[13:00] <deryck> benji, I'm looking, but I think you need the reviewer to comment again with "approve"
[13:00] <benji> k
[13:00] <deryck> benji, yeah, so lifeless needs to select approve at the comment form, too.
[13:20] <wgrant> stub: Hi. Can I please have a DB review of https://code.edge.launchpad.net/~wgrant/launchpad/link-uploaded-ddebs/+merge/29669 ?
[13:29] <lifeless> its a lp land bug
[13:29] <lifeless> benji: ^
[13:29] <benji> :(
[13:30] <benji> lifeless: is there a work-around?
[13:30] <benji> or shall I hack lp-land :)
[13:30] <lifeless> yes, you get me to tick the button which I'm doing
[13:30] <benji> heh; ok
[13:30] <lifeless> please do fix lp-land, the bug was filed tuesday or so by flacoste
[13:31] <benji> by "hack" I meant "comment out the check"
[13:31] <lifeless> we need to fix the tool
[13:31] <lifeless> :)
[13:31] <benji> I suspect bzr people will be faster at fixing it than I
[13:36] <lifeless> no
[13:36] <lifeless> by which I mean
[13:36] <lifeless> its a lp toolchain bug
[13:37] <lifeless> not a bzr toolchain bug
[13:37] <lifeless> you may find some glue knowledge needed
[13:37] <lifeless> but its all about lp api's and lp team policy
[13:37] <lifeless> plus, yak shaving is good, its how we fix friction.
[13:38] <bigjools> wgrant: can you get someone else to finish reviewing your branch please, I won't have time :(
[13:38] <wgrant> bigjools: OK, sure.
[13:38] <wgrant> Thanks.
[13:39] <bigjools> sorry - it's kinda hectic for me at the moment
[13:39] <wgrant> That's fine. I just wanted your approval for general direction. I wasn't expecting a proper review.
[13:39] <bigjools> wgrant: don't forget to file a bug about fixing the tests though
[13:40] <bigjools> the basic doc tests are fine, we just don't want unit tests in them
[13:40] <wgrant> Hmm.
[13:40] <bigjools> and also I would prefer unit tests instead of uploading whole new packages where possible
[13:40] <wgrant> Oh, certainly.
[13:41] <wgrant> But archiveuploader isn't awesome for that at the moment.
[13:41] <bigjools> if you add unit tests to doctests, various people like jml will hunt you down :)
[13:41] <bigjools> you need to factor out your new code
[13:41] <bigjools> then it becomes testable :)
[13:41] <wgrant> I need to move around some other stuff before I can do that.
[13:41] <bigjools> ok
[13:42] <bigjools> like I said, feel free to file a bug about it
[13:42]  * bigjools comments on the MP
[13:42] <wgrant> I may get bored tomorrow and rip it to pieces anyway.
[13:43] <wgrant> bigjools: What do you think about stub's suggestion on the MP?
[13:44] <wgrant> I'm not sure that should be done, given that we have no way to delete BPRs yet, and any such functionality would require massive changes in lots of other places too.
[13:44] <bigjools> I've not seen it
[13:44] <wgrant> He suggests ON DELETE SET NULL on the new column.
[13:45] <bigjools> it won't harm
[13:45] <bigjools> although as you say we've no way of deleting them
[13:47] <wgrant> Right.
[13:48] <wgrant> bigjools: Are you moving to the Bugs and Code 'doctests are for documentation: kill them all and get full unit test coverage instead' policy, or do you still feel that some stuff belongs in them?
[13:48] <wgrant> (I'm wondering how aggressive I should be in my culling)
[13:49] <bigjools> wgrant: it's going to be an attritional war - although we still are yet to define exactly what documentation is, for a doctest
[13:49]  * gary_poster hopes that cullers actually take a moment to verify that a given doctest is not reasonably valuable documentation as they are culling
[13:49] <gary_poster> but, yeah, what that means is up for grabs
[13:50] <gary_poster> salgado, one of the XXXs is "<!-- XXX Probably make this in overrides for ISimpleReadContainer.  Safer: register for all apidoc containers, one way or another -->"
[13:50] <wgrant> gary_poster: Of course.
[13:50] <gary_poster> I understand the second half of that
[13:50] <gary_poster> but not the overrides part
[13:50] <gary_poster> why would an override be better?
[13:52] <salgado> gary_poster, I was hoping you'd tell me that as you wrote the comment. :P
[13:53] <gary_poster> lol :-)
[13:53] <gary_poster> uh, ok, I'll stare at it and see if I think I was on drugs at the time
[13:58] <gary_poster> salgado: I think I expected that this was going to have to go in overrides.  Since it works without being in overrides, this is a "never mind..."
[14:00] <salgado> gary_poster, cool.  the other XXX is my doing and I think we can turn it into a regular comment unless you have another solution for that
[14:00] <gary_poster> salgado: completely agree.  This is what I was writing in my review about that other one:
[14:00] <gary_poster> I suggest that this is not an XXX.  To that point, this is not "Copied" but "Modified" (the "for" is different, and that's the key point).
[14:10] <lifeless> I can has review ? https://code.edge.launchpad.net/~lifeless/launchpad/malone/+merge/30507
[14:12] <lifeless> flacoste: hi
[14:12] <flacoste> hi lifeless (hopping on a call)
[14:12] <deryck> lifeless, I'm juggling two private conversations and trying to get my own branch landed, if anyone else can help, that might be better.  if not, I'll catch it shortly.
[14:12] <lifeless> deryck: kk
[14:12] <lifeless> rockstar: ^ is the one
[16:40] <bac> jtv: what do you need to have reviewed?  i don't see anything on +activereviews
[16:40] <jtv> bac: robert just took it
[16:40] <bac> ok
[16:41] <bac> jtv: update the topic?
[16:41] <bac> :)
[16:41] <jtv> bac: it all happened in seconds...
[20:47] <lifeless> hi
[20:47] <lifeless> anyone up for an incremental +1  ? http://pastebin.com/N64WnzaU
[20:47] <lifeless> thats the test fallout fixes for my faster-bugsearch branch
[20:53] <lifeless> rockstar: so, doctests have poor isolation
[20:53] <lifeless> 'sensitive' is the phrase that comes to mind
[20:54] <rockstar> lifeless, also, doctests tend to do runaway tests, where it ends up testing way too much.
[20:54] <lifeless> yeah
[20:54] <rockstar> lifeless, it's fine.
[20:54] <rockstar> Er, your patch, not runaway tests.
[20:54] <lifeless> thanks :)
[20:55] <rockstar> And as terribly as the API for out current browser unit tests are, abentley and I have done a pretty good job of making sure the test only really tests one thing.
[20:56] <lifeless> \o/
[20:56] <lifeless> I had some windmill guff, I think its my test environment noise
[20:56] <rockstar> lifeless, the windmill tests all leave threads behind.
[20:56] <rockstar> It's just noise though, methinks.
[20:56] <lifeless> this was failures - but its running in ff outside the vm
[20:57] <lifeless> I highly doubt its glued together right ;)
[20:57] <rockstar> lifeless, yeah, the windmill tests aren't the best.  mars has done a good job of making them much better though.