=== 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 [12:40] danilos, you are getting something reviewed, sweet :-) [12:44] gary_poster, well, MP is up for review, bastard OCR has not showed up today [12:44] (gmb that is :)) [12:44] lol [12:44] gary_poster, I wouldn't mind you reviewing it if you want to see how much you hate my approach :) [12:44] lol, sure, send me the link [12:46] gary_poster, https://code.launchpad.net/~danilo/launchpad/bug728370-nondirect-subs/+merge/57293 [12:46] cool, on it danilo [12:46] gary_poster, just so you don't get scared, more than half of it is tests, thanks :) [12:46] heh, ok :-) [12:48] i too am waiting on the bastard OCR: https://code.launchpad.net/~bac/launchpad/bug-753965/+merge/57262 [12:53] 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:54] I assume leonard is not showing up for his review duties? :) removing him from OCR [13:08] I'll do a review if anyone needs it. [13:19] 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] gary_poster, heh, yeah, thanks :) [13:20] 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. < https://launchpad.net/bugs/750567 > [13:20] 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] benji: please see above if you want to review. [13:20] bac: cool, starting now [13:20] benji, maybe so. [13:21] I'd be glad to look if you want. [13:21] benji: ignore the lint as i'm fixing it now [13:21] bac: k [13:21] It instantly disappears now, but the instant is a trifle too soon :-) [13:25] gary_poster: right, I think I was overly aggressive when the user clicks OK [13:25] gary_poster: i'm not sure what i was thinking, but trizpug is on the 28th, not this thursday [13:25] bac, cool. I hopefully will have a new baby by then :-) [13:26] bac benji danilos I'm about to look at kanban board [13:26] call in 4 [13:26] bac: on line 76 of the diff did you mean to use the self.contents as the message describing the failure? [13:26] k [13:26] and lest I forget, I'm out this morning [13:26] starting 9:30-ish till lunchish [13:26] benji: no, i'm sure that is left over debugging [13:26] k [13:26] benji: sorry [13:26] where 9:30-ish is an hour-ish from now-ish :-) [13:26] i hate when i leave those things in [13:27] bac: it's the worst thing that has ever happened to me [13:27] i know! [13:27] lol [13:27] you've led a charmed life [13:27] heh [13:37] gary_poster: two leave reqeusts posted on canonicaladmin [13:40] fwiw, pastebin function: http://pastebin.ubuntu.com/593099/ [13:53] approved bac [13:56] 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] 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] Like I said, that's tiny, but it has confused me just a bit. [13:56] gary_poster, heh, exactly :) [13:57] gary_poster, I'll be happy to fix it (you can probably assume a lot of copy-pasting has gone on :) [13:57] heh, cool [14:18] 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] 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:19] heh ok cool :-) [14:19] 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:20] 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:22] gary_poster, right, thanks again for the review :) [14:26] np [14:26] talk to you all later [15:08] bac: through pilot error I lost my review of your branch, reconstructing now [15:09] benji: ok [15:54] bac: review now done for the second time: https://code.launchpad.net/~bac/launchpad/bug-753965/+merge/57262 [15:54] thx [15:55] 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] benji: thanks. i appreciate whatever suggestion you have. it got out of control [15:56] I think I found the sanest way to do what we want there. Who reviews the code suggested by the reviewer? :) [16:38] 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 [17:30] benji: the CustomTestLoad rocks -- thanks! [17:32] excellent === Ursinha is now known as Ursinha-afk === Ursinha-afk is now known as Ursinha === Ursinha is now known as Ursinha-afk [20:50] benji, you up for short call [20:51] gary_poster: sure [20:51] thank you === Ursinha-afk is now known as Ursinha