| === daniloff is now known as danilos | ||
| deryck | mrevell, hi. Can you do a text review for me? | 12:33 |
|---|---|---|
| mrevell | Sure deryck | 12:33 |
| 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:34 | |
| mrevell | deryck, I meant to tag that as "text" | 12:46 |
| 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:47 |
| mrevell | Heh, just re-read it and noticed that I didn't finish typng my comment | 12:48 |
| mrevell | deryck, Sorry, please refresh. | 12:49 |
| deryck | ok, refreshing now. sorry too many xchat tabs today. | 12:50 |
| deryck | mrevell, yeah, I like that better. I'll fix and submit a new screenshot for you now. | 12:51 |
| 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:58 |
| 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" | 12:59 |
| 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:00 |
| wgrant | stub: Hi. Can I please have a DB review of https://code.edge.launchpad.net/~wgrant/launchpad/link-uploaded-ddebs/+merge/29669 ? | 13:20 |
| lifeless | its a lp land bug | 13:29 |
| lifeless | benji: ^ | 13:29 |
| benji | :( | 13:29 |
| 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:30 |
| 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:31 |
| lifeless | no | 13:36 |
| lifeless | by which I mean | 13:36 |
| lifeless | its a lp toolchain bug | 13:36 |
| 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:37 |
| 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: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 | ||
| 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:39 |
| 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:40 |
| 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:41 |
| 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:42 |
| wgrant | bigjools: What do you think about stub's suggestion on the MP? | 13:43 |
| 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:44 |
| bigjools | it won't harm | 13:45 |
| bigjools | although as you say we've no way of deleting them | 13:45 |
| wgrant | Right. | 13:47 |
| 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:48 |
| 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:49 |
| 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:50 |
| salgado | gary_poster, I was hoping you'd tell me that as you wrote the comment. :P | 13:52 |
| 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:53 |
| 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..." | 13:58 |
| 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:00 |
| === flacoste_afk is now known as flacoste | ||
| lifeless | I can has review ? https://code.edge.launchpad.net/~lifeless/launchpad/malone/+merge/30507 | 14:10 |
| 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 | 14: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 | ||
| 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:40 |
| bac | jtv: 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 | ||
| bac | :) | 16:41 |
| jtv | bac: 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 | ||
| 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:47 |
| lifeless | rockstar: so, doctests have poor isolation | 20:53 |
| lifeless | 'sensitive' is the phrase that comes to mind | 20:53 |
| 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:54 |
| 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:55 |
| 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:56 |
| 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. | 20:57 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!