| === Ursinha-afk is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| gary_poster | danilos, you are getting something reviewed, sweet :-) | 12:40 |
|---|---|---|
| danilos | gary_poster, well, MP is up for review, bastard OCR has not showed up today | 12:44 |
| danilos | (gmb that is :)) | 12:44 |
| gary_poster | lol | 12:44 |
| danilos | gary_poster, I wouldn't mind you reviewing it if you want to see how much you hate my approach :) | 12:44 |
| gary_poster | lol, sure, send me the link | 12:44 |
| danilos | gary_poster, https://code.launchpad.net/~danilo/launchpad/bug728370-nondirect-subs/+merge/57293 | 12:46 |
| gary_poster | cool, on it danilo | 12:46 |
| danilos | gary_poster, just so you don't get scared, more than half of it is tests, thanks :) | 12:46 |
| gary_poster | heh, ok :-) | 12:46 |
| bac | i too am waiting on the bastard OCR: https://code.launchpad.net/~bac/launchpad/bug-753965/+merge/57262 | 12:48 |
| danilos | bac, I'd be happy to help in an hour or two if that's not too late for you? if you are blocked, I'd take a look now | 12:53 |
| danilos | I assume leonard is not showing up for his review duties? :) removing him from OCR | 12:54 |
| benji | I'll do a review if anyone needs it. | 13:08 |
| gary_poster | danilos, fwiw this looks great so far. I'm on line 400+ of the diff, and I've run the test and the demo. Your test console is preeeeettttty ;-) | 13:19 |
| danilos | gary_poster, heh, yeah, thanks :) | 13:19 |
| benji | gary_poster: I bet the new "overlay closes immediately" bug is a result of https://code.launchpad.net/~benji/launchpad/bug-750573-move-overlay (the fix for bug 750567) | 13:20 |
| _mup_ | Bug #750567: Structural subscription overlay doesn't instantly disappear. <qa-ok> <story-better-bug-notification> <Launchpad itself:Fix Released by benji> < https://launchpad.net/bugs/750567 > | 13:20 |
| danilos | gary_poster, I've got a prototype rendering and direct sub as well, just need to polish it and add tests for the display (wanted to make sure I can render what I've got :)) | 13:20 |
| bac | benji: please see above if you want to review. | 13:20 |
| benji | bac: cool, starting now | 13:20 |
| gary_poster | benji, maybe so. | 13:20 |
| benji | I'd be glad to look if you want. | 13:21 |
| bac | benji: ignore the lint as i'm fixing it now | 13:21 |
| benji | bac: k | 13:21 |
| gary_poster | It instantly disappears now, but the instant is a trifle too soon :-) | 13:21 |
| benji | gary_poster: right, I think I was overly aggressive when the user clicks OK | 13:25 |
| bac | gary_poster: i'm not sure what i was thinking, but trizpug is on the 28th, not this thursday | 13:25 |
| gary_poster | bac, cool. I hopefully will have a new baby by then :-) | 13:25 |
| gary_poster | bac benji danilos I'm about to look at kanban board | 13:26 |
| gary_poster | call in 4 | 13:26 |
| benji | bac: on line 76 of the diff did you mean to use the self.contents as the message describing the failure? | 13:26 |
| benji | k | 13:26 |
| gary_poster | and lest I forget, I'm out this morning | 13:26 |
| gary_poster | starting 9:30-ish till lunchish | 13:26 |
| bac | benji: no, i'm sure that is left over debugging | 13:26 |
| benji | k | 13:26 |
| bac | benji: sorry | 13:26 |
| gary_poster | where 9:30-ish is an hour-ish from now-ish :-) | 13:26 |
| bac | i hate when i leave those things in | 13:26 |
| benji | bac: it's the worst thing that has ever happened to me | 13:27 |
| bac | i know! | 13:27 |
| gary_poster | lol | 13:27 |
| bac | you've led a charmed life | 13:27 |
| benji | heh | 13:27 |
| bac | gary_poster: two leave reqeusts posted on canonicaladmin | 13:37 |
| danilos | fwiw, pastebin function: http://pastebin.ubuntu.com/593099/ | 13:40 |
| gary_poster | approved bac | 13:53 |
| gary_poster | danilos, super small, I'm not sure what you mean by "short (just-enough)" in your test comments (e.g., "Gather short (just-enough) subscription records for all assignments."). It makes me think I don't know what is going on. :-) | 13:56 |
| gary_poster | I suspect it is simple--maybe you mean that you are dumping all of the information that you don't need from the personsubscriptioninfo data?--but I also suspect that just dropping that phrase will not lose any important information to the reader and make it easier to understand. | 13:56 |
| gary_poster | Like I said, that's tiny, but it has confused me just a bit. | 13:56 |
| danilos | gary_poster, heh, exactly :) | 13:56 |
| danilos | gary_poster, I'll be happy to fix it (you can probably assume a lot of copy-pasting has gone on :) | 13:57 |
| gary_poster | heh, cool | 13:57 |
| gary_poster | danilos, approved. The only other thing of smallest value I mentioned was questioning your switch/case indentation. blah. :-) If you want to leave it the way it is that's fine. I also asked about whether you could reduce cut & paste in the tests, but I looked for a way, and on a quick look I didn't see a compelling option. So, you can land as-is if you like. | 14:18 |
| danilos | gary_poster, you are not the first one to question it, emacs espresso mode defaults to that, I need to look into configuring it to indent "properly" :) | 14:18 |
| gary_poster | heh ok cool :-) | 14:19 |
| danilos | gary_poster, reducing the cut and paste is non-trivial because messages (at least) differ, and the tests as such have allowed me to do some code refactoring | 14:19 |
| gary_poster | danilos, cool. yeah, and it's not even just messages. test structure themselves change. In the example I looked at, there were five functions with the same name, two of which were similar enough for merging. Not worth it. | 14:20 |
| danilos | gary_poster, right, thanks again for the review :) | 14:22 |
| gary_poster | np | 14:26 |
| gary_poster | talk to you all later | 14:26 |
| benji | bac: through pilot error I lost my review of your branch, reconstructing now | 15:08 |
| bac | benji: ok | 15:09 |
| benji | bac: review now done for the second time: https://code.launchpad.net/~bac/launchpad/bug-753965/+merge/57262 | 15:54 |
| bac | thx | 15:54 |
| benji | bac: be forwarned, I went on a small tangent about the test suite construction, but I included a patch, so it shouldn't take much effort to integrate it | 15:55 |
| bac | benji: thanks. i appreciate whatever suggestion you have. it got out of control | 15:55 |
| benji | I think I found the sanest way to do what we want there. Who reviews the code suggested by the reviewer? :) | 15:56 |
| danilos | gary_poster, I've got to run, my branch up at lp:~danilo/launchpad/bug728370-direct-subs though there's still some work to get even the non-pretty descriptions printout landed (mainly refactoring and tests) | 16:38 |
| * danilos -> off | 16:38 | |
| bac | benji: the CustomTestLoad rocks -- thanks! | 17:30 |
| benji | excellent | 17:32 |
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
| gary_poster | benji, you up for short call | 20:50 |
| benji | gary_poster: sure | 20:51 |
| gary_poster | thank you | 20:51 |
| === Ursinha-afk is now known as Ursinha | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!