=== daniloff is now known as danilos [12:33] mrevell, hi. Can you do a text review for me? [12:33] Sure deryck [12:34] mrevell, https://code.edge.launchpad.net/~deryck/launchpad/better-dupe-handling-ui/+merge/30309 (see last comment on MP) [12:34] mrevell, thanks! [12:34] * mrevell looks [12:46] deryck, I meant to tag that as "text" [12:47] 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] mrevell, ok, thanks, looking now... [12:48] Heh, just re-read it and noticed that I didn't finish typng my comment [12:49] deryck, Sorry, please refresh. [12:50] ok, refreshing now. sorry too many xchat tabs today. [12:51] mrevell, yeah, I like that better. I'll fix and submit a new screenshot for you now. [12:58] 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] mrevell, new screenie up on the MP. [12:59] * mrevell looks [12:59] It's marked as "Approved" but the only vote was "Needs Fixing" [13:00] benji, I'm looking, but I think you need the reviewer to comment again with "approve" [13:00] k [13:00] benji, yeah, so lifeless needs to select approve at the comment form, too. [13:20] stub: Hi. Can I please have a DB review of https://code.edge.launchpad.net/~wgrant/launchpad/link-uploaded-ddebs/+merge/29669 ? [13:29] its a lp land bug [13:29] benji: ^ [13:29] :( [13:30] lifeless: is there a work-around? [13:30] or shall I hack lp-land :) [13:30] yes, you get me to tick the button which I'm doing [13:30] heh; ok [13:30] please do fix lp-land, the bug was filed tuesday or so by flacoste [13:31] by "hack" I meant "comment out the check" [13:31] we need to fix the tool [13:31] :) [13:31] I suspect bzr people will be faster at fixing it than I [13:36] no [13:36] by which I mean [13:36] its a lp toolchain bug [13:37] not a bzr toolchain bug [13:37] you may find some glue knowledge needed [13:37] but its all about lp api's and lp team policy [13:37] plus, yak shaving is good, its how we fix friction. [13:38] wgrant: can you get someone else to finish reviewing your branch please, I won't have time :( [13:38] bigjools: OK, sure. [13:38] Thanks. === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [13:39] sorry - it's kinda hectic for me at the moment [13:39] That's fine. I just wanted your approval for general direction. I wasn't expecting a proper review. [13:39] wgrant: don't forget to file a bug about fixing the tests though [13:40] the basic doc tests are fine, we just don't want unit tests in them [13:40] Hmm. [13:40] and also I would prefer unit tests instead of uploading whole new packages where possible [13:40] Oh, certainly. [13:41] But archiveuploader isn't awesome for that at the moment. [13:41] if you add unit tests to doctests, various people like jml will hunt you down :) [13:41] you need to factor out your new code [13:41] then it becomes testable :) [13:41] I need to move around some other stuff before I can do that. [13:41] ok [13:42] like I said, feel free to file a bug about it [13:42] * bigjools comments on the MP [13:42] I may get bored tomorrow and rip it to pieces anyway. [13:43] bigjools: What do you think about stub's suggestion on the MP? [13:44] 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] I've not seen it [13:44] He suggests ON DELETE SET NULL on the new column. [13:45] it won't harm [13:45] although as you say we've no way of deleting them [13:47] Right. [13:48] 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] (I'm wondering how aggressive I should be in my culling) [13:49] 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] but, yeah, what that means is up for grabs [13:50] salgado, one of the XXXs is "" [13:50] gary_poster: Of course. [13:50] I understand the second half of that [13:50] but not the overrides part [13:50] why would an override be better? [13:52] gary_poster, I was hoping you'd tell me that as you wrote the comment. :P [13:53] lol :-) [13:53] uh, ok, I'll stare at it and see if I think I was on drugs at the time [13:58] 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] 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] salgado: completely agree. This is what I was writing in my review about that other one: [14:00] 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). === flacoste_afk is now known as flacoste [14:10] I can has review ? https://code.edge.launchpad.net/~lifeless/launchpad/malone/+merge/30507 [14:12] flacoste: hi [14:12] hi lifeless (hopping on a call) [14:12] 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] deryck: kk [14:12] rockstar: ^ is the one === jtv changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [wgrant, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:40] jtv: what do you need to have reviewed? i don't see anything on +activereviews [16:40] bac: robert just took it [16:40] ok [16:41] jtv: update the topic? === jtv changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:41] :) [16:41] bac: it all happened in seconds... === deryck is now known as deryck[lunch] === deryck[lunch] is now known as deryck === adiroiban is now known as Ow2 [20:47] hi [20:47] anyone up for an incremental +1 ? http://pastebin.com/N64WnzaU [20:47] thats the test fallout fixes for my faster-bugsearch branch [20:53] rockstar: so, doctests have poor isolation [20:53] 'sensitive' is the phrase that comes to mind [20:54] lifeless, also, doctests tend to do runaway tests, where it ends up testing way too much. [20:54] yeah [20:54] lifeless, it's fine. [20:54] Er, your patch, not runaway tests. [20:54] thanks :) [20:55] 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] \o/ [20:56] I had some windmill guff, I think its my test environment noise [20:56] lifeless, the windmill tests all leave threads behind. [20:56] It's just noise though, methinks. [20:56] this was failures - but its running in ff outside the vm [20:57] I highly doubt its glued together right ;) [20:57] lifeless, yeah, the windmill tests aren't the best. mars has done a good job of making them much better though.