=== daniloff is now known as danilos
deryckmrevell, hi.  Can you do a text review for me?12:33
mrevellSure deryck12:33
deryckmrevell, https://code.edge.launchpad.net/~deryck/launchpad/better-dupe-handling-ui/+merge/30309 (see last comment on MP)12:34
deryckmrevell, thanks!12:34
* mrevell looks12:34
mrevellderyck, I meant to tag that as "text"12:46
mrevellderyck, 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
deryckmrevell, ok, thanks, looking now...12:47
mrevellHeh, just re-read it and noticed that I didn't finish typng my comment12:48
mrevellderyck, Sorry, please refresh.12:49
deryckok, refreshing now.  sorry too many xchat tabs today.12:50
deryckmrevell, yeah, I like that better.  I'll fix and submit a new screenshot for you now.12:51
benjican 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:58
deryckmrevell, new screenie up on the MP.12:59
* mrevell looks12:59
benjiIt's marked as "Approved" but the only vote was "Needs Fixing"12:59
deryckbenji, I'm looking, but I think you need the reviewer to comment again with "approve"13:00
deryckbenji, yeah, so lifeless needs to select approve at the comment form, too.13:00
wgrantstub: Hi. Can I please have a DB review of https://code.edge.launchpad.net/~wgrant/launchpad/link-uploaded-ddebs/+merge/29669 ?13:20
lifelessits a lp land bug13:29
lifelessbenji: ^13:29
benjilifeless: is there a work-around?13:30
benjior shall I hack lp-land :)13:30
lifelessyes, you get me to tick the button which I'm doing13:30
benjiheh; ok13:30
lifelessplease do fix lp-land, the bug was filed tuesday or so by flacoste13:30
benjiby "hack" I meant "comment out the check"13:31
lifelesswe need to fix the tool13:31
benjiI suspect bzr people will be faster at fixing it than I13:31
lifelessby which I mean13:36
lifelessits a lp toolchain bug13:36
lifelessnot a bzr toolchain bug13:37
lifelessyou may find some glue knowledge needed13:37
lifelessbut its all about lp api's and lp team policy13:37
lifelessplus, yak shaving is good, its how we fix friction.13:37
bigjoolswgrant: can you get someone else to finish reviewing your branch please, I won't have time :(13:38
wgrantbigjools: OK, sure.13:38
=== 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
bigjoolssorry - it's kinda hectic for me at the moment13:39
wgrantThat's fine. I just wanted your approval for general direction. I wasn't expecting a proper review.13:39
bigjoolswgrant: don't forget to file a bug about fixing the tests though13:39
bigjoolsthe basic doc tests are fine, we just don't want unit tests in them13:40
bigjoolsand also I would prefer unit tests instead of uploading whole new packages where possible13:40
wgrantOh, certainly.13:40
wgrantBut archiveuploader isn't awesome for that at the moment.13:41
bigjoolsif you add unit tests to doctests, various people like jml will hunt you down :)13:41
bigjoolsyou need to factor out your new code13:41
bigjoolsthen it becomes testable :)13:41
wgrantI need to move around some other stuff before I can do that.13:41
bigjoolslike I said, feel free to file a bug about it13:42
* bigjools comments on the MP13:42
wgrantI may get bored tomorrow and rip it to pieces anyway.13:42
wgrantbigjools: What do you think about stub's suggestion on the MP?13:43
wgrantI'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
bigjoolsI've not seen it13:44
wgrantHe suggests ON DELETE SET NULL on the new column.13:44
bigjoolsit won't harm13:45
bigjoolsalthough as you say we've no way of deleting them13:45
wgrantbigjools: 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:48
bigjoolswgrant: it's going to be an attritional war - although we still are yet to define exactly what documentation is, for a doctest13:49
* gary_poster hopes that cullers actually take a moment to verify that a given doctest is not reasonably valuable documentation as they are culling13:49
gary_posterbut, yeah, what that means is up for grabs13:49
gary_postersalgado, 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
wgrantgary_poster: Of course.13:50
gary_posterI understand the second half of that13:50
gary_posterbut not the overrides part13:50
gary_posterwhy would an override be better?13:50
salgadogary_poster, I was hoping you'd tell me that as you wrote the comment. :P13:52
gary_posterlol :-)13:53
gary_posteruh, ok, I'll stare at it and see if I think I was on drugs at the time13:53
gary_postersalgado: 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..."13:58
salgadogary_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 that14:00
gary_postersalgado: completely agree.  This is what I was writing in my review about that other one:14:00
gary_posterI 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:00
=== flacoste_afk is now known as flacoste
lifelessI can has review ? https://code.edge.launchpad.net/~lifeless/launchpad/malone/+merge/3050714:10
lifelessflacoste: hi14:12
flacostehi lifeless (hopping on a call)14:12
derycklifeless, 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
lifelessderyck: kk14:12
lifelessrockstar: ^ is the one14:12
=== 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
bacjtv: what do you need to have reviewed?  i don't see anything on +activereviews16:40
jtvbac: robert just took it16:40
bacjtv: update the topic?16:41
=== 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
jtvbac: it all happened in seconds...16:41
=== deryck is now known as deryck[lunch]
=== deryck[lunch] is now known as deryck
=== adiroiban is now known as Ow2
lifelessanyone up for an incremental +1  ? http://pastebin.com/N64WnzaU20:47
lifelessthats the test fallout fixes for my faster-bugsearch branch20:47
lifelessrockstar: so, doctests have poor isolation20:53
lifeless'sensitive' is the phrase that comes to mind20:53
rockstarlifeless, also, doctests tend to do runaway tests, where it ends up testing way too much.20:54
rockstarlifeless, it's fine.20:54
rockstarEr, your patch, not runaway tests.20:54
lifelessthanks :)20:54
rockstarAnd 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:55
lifelessI had some windmill guff, I think its my test environment noise20:56
rockstarlifeless, the windmill tests all leave threads behind.20:56
rockstarIt's just noise though, methinks.20:56
lifelessthis was failures - but its running in ff outside the vm20:56
lifelessI highly doubt its glued together right ;)20:57
rockstarlifeless, yeah, the windmill tests aren't the best.  mars has done a good job of making them much better though.20:57

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!