=== EdwinGrubbs changed the topic of #launchpad-reviews to:  On call: - || Reviewing: leonardr || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== EdwinGrubbs changed the topic of #launchpad-reviews to:  On call: - || Reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[03:02] <lifeless> https://code.edge.launchpad.net/~lifeless/launchpad/bug-627701/+merge/37094
[03:02] <lifeless> if anyone is up for that, it will allow very specific timeouts.
[03:06] <mwhudson> lifeless: "def _get_request_timeout" ignores the 'now' parameter
[03:08] <mwhudson> in fact, given what it seems to do, the tests of it could be precise
[03:08] <mwhudson> abs(x-y) < 0.001 always creeps me out
[04:46] <lifeless> mwhudson: floats creep me out
[04:48] <rockstar> lifeless, even the root beer kind?
[04:49] <lifeless> grah yes ;)
[04:50] <thumper> root beer is awful
[04:50] <lifeless> mwhudson: so was that 'please land with the now parameter removed'
[04:50] <lifeless> mwhudson: or 'please find another reviewer I've gone to get beer'?
[04:50] <rockstar> thumper, I've had what kiwis call "root beer." It is a bad forgery.
[04:58] <lifeless> ok, looks like I needs a reviewer for https://code.edge.launchpad.net/~lifeless/launchpad/bug-627701/+merge/37094 (updated)
[05:05] <lifeless> mwhudson: if you do return, and see this, please clickyclicky ;)
=== Ursinha-bbl is now known as Ursinha-afk
=== noodles775 changed the topic of #launchpad-reviews to:  On call: noodles775 || Reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[08:42] <noodles775> StevenK: ah, this is the same branch that we were chatting about earlier... should I wait, or start working through it?
[08:43] <noodles775> StevenK: btw, have you tried `bzr lp-propose-merge`? It will create a pretty handy template for your MPs.
[08:45] <StevenK> noodles775: There's two: https://code.edge.launchpad.net/~stevenk/launchpad/classy-packagecloner/+merge/36963 is the one in the topic
[08:45] <noodles775> GReat, I'll start on that one then :)
[09:06] <noodles775> StevenK: I'm just looking at the class-packagecloner branch - did you have a pre-imp. call with someone (bigjools?) about it? I don't understand why you're wanting to use instance variables to store state on a utility...
[09:07] <StevenK> noodles775: I mentioned it to Julian, yes. It just seems ... messy
[09:08] <noodles775> StevenK: yes, but I don't think storing state for an individual call on a shared utility object is safe? Remember, two separate callsites might be using the utility at the same time (which is why - I assume - all the params were being passed in).
[09:09] <StevenK> noodles775: The only entry point is still clonePackages() and they get their own instance, don't they?
[09:09] <noodles775> StevenK: no, getUtility(IPackageCloner) will return the same object each time (AFAIUI).
[09:10]  * noodles775 checks the source for getUtility.
[09:10] <StevenK> noodles775: Okay, if that's the case, then yes, this branch is a bad idea.
=== allenap changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: StevenK, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[09:12] <allenap> Morning noodles775 :)
[09:13] <noodles775> Hi allenap!
=== StevenK changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[09:53] <henninge> danilos: https://code.edge.launchpad.net/~henninge/launchpad/recife-bug-611674-convert-imports/+merge/37108
[09:53] <henninge> danilos: I still have to push lint changes
[09:54] <henninge> pushed ;)
[09:57] <danilos> henninge, thanks, I'll take a look at it as soon as I am back, got to get out now :)
[10:02] <henninge> danilos: np
=== allenap changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: -, Edwin || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== StevenK changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: -, Edwin || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[11:15] <noodles775> Hi henninge, can I interest you in another UI review? http://people.canonical.com/~michaeln/tmp/649559-blacklisting-with-comments.ogv
[11:15] <noodles775> Actual MP here: https://code.edge.launchpad.net/~michael.nelson/launchpad/649559-ajax-comment-on-dsdiff-2/+merge/37114
=== noodles775 changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: StevenK, Edwin || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[11:16] <henninge> noodles775: sure, but I have to finish something first.
[11:17]  * henninge watches screencast right away, though ... ;)
[11:17] <noodles775> henninge: no problem (or if you're busy, salgado will be around a bit later too). Thanks.
[11:17] <noodles775> Ah :)
[11:18] <henninge> noodles775: hm, aren't comments always sorted from oldest to newest in LP? We should stick to that.
[11:20] <noodles775> henninge: I wasn't sure in this case - it is more like a wall... as a user, the information I want is the most recent comments. But it's easy to switch.
[11:20] <henninge> noodles775: also, in the same vein, "Add comment" is usually placed at the end of the list of comments.
[11:20] <henninge> noodles775: Did I mention that this looks great! ;-D
[11:21] <henninge> noodles775: sorry for starting off with the negative things ... ;)
[11:21] <noodles775> henninge: yes, if the comments are going oldest to newest, then of course it would be at the end. It's only there because I did it more like a wall.
[11:21] <noodles775> No problem! Thanks for the feedback.
[11:21] <henninge> noodles775: well, I don't see how this case is different from other commenting we have in LP
[11:22] <noodles775> henninge: The biggest difference, IMO, is that we're displaying lists of comments for multiple objects on the one page. Ideally, if I had more time, we'd only be showing the 5 latest comments for each (in the details section).
[11:22] <henninge> noodles775: mp's have a "Add a review or comment" button on top of the comments that scrolls you down to the end.
[11:23] <henninge> dunno why bugs don't.
[11:24] <henninge> noodles775: I am not opposed to introducing a "wall" concept for short comments - but they should look different to mark the difference in behaviour.
[11:24] <noodles775> james_w: if you're around and have a minute, would you also prefer standard oldest to newest comments for the differences (see above screencast). I've currently got them with the newest first, assuming that's the info that will be needed, but it's easy to switch back.
[11:26] <noodles775> henninge: If the heading was "Most recent comments" instead of "Comments" (and later we added a link to the full details for a particular difference with all comments) would that help differentiate?
[11:26] <noodles775> (er, and we only showed the 5 most recent on this page?)
[11:28] <henninge> noodles775: That would probably help but I still think people will get confused about the ordering.
[11:29] <henninge> noodles775: I actually think the reverse ordering is preferrable
[11:29] <noodles775> henninge: yeah, perhaps. Let's see what james_w or other people who will be using this say. I'm happy to change it - it's an easy switch.
[11:30] <henninge> noodles775: I don't know if it has been discussed before but maybe you could post your screencast to the list and start a discussion to adopt this for all our comments?
[11:31] <henninge> but maybe that is not always wanted ...
[11:31] <henninge> hm
[11:34] <noodles775> henninge: no, I think a merge proposal is a story... it should be ordered from oldest to newest.
[11:35] <noodles775> The few comments that will ever be used for these differences are things like "Waiting for version 2.5".
[11:41] <henninge> noodles775: I put that all in a review comment. Not approved until we decide on a way to go with this.
[11:41] <henninge> noodles775: but thanks a lot for the great work! ;-)
[11:41] <noodles775> henninge: yep, that's great (and I don't mind which way... just what works for the people using it). Thanks!
=== henninge changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: StevenK, Edwin || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[12:17] <henninge> noodles775, allenap: I have some mechanical removal of "canonical.launchpad.interfaces" imports here: https://code.edge.launchpad.net/~henninge/launchpad/devel-remove-c-l-i/+merge/37116
[12:18] <noodles775> oooh, exciting :)
=== allenap changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: StevenK, henninge || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== allenap changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: StevenK, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== mrevell is now known as mrevell-lunch
=== noodles775 changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: lunch, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== mrevell-lunch is now known as mrevell
=== noodles775 changed the topic of #launchpad-reviews to:  On call: noodles775, allenap || Reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[13:44] <james_w> noodles775: I think newest first makes some sense here, but I don't know if consistency should win out
[13:46] <noodles775> OK, thanks james_w - let's see what the ui reviewers (henninge and sinzui) decide.
=== noodles775 changed the topic of #launchpad-reviews to:  On call: allenap || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[14:38] <noodles775> sinzui: Hi! When you've time, can you pls take a look at the following (no rush): https://code.edge.launchpad.net/~michael.nelson/launchpad/649559-ajax-comment-on-dsdiff-2/+merge/37114
[14:38]  * sinzui will in 20 minutes
[14:38] <noodles775> Great, thanks sinzui
=== allenap changed the topic of #launchpad-reviews to:  On call: allenap || Reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[15:06] <leonardr> EdwinGrubbs: a tiny follow-up to the branch you reviewed yesterday. i added a doctest that benji wrote to complement the pagetests:
[15:06] <leonardr> http://pastebin.ubuntu.com/503270/
[15:06] <leonardr> take a look and i'll land the branch
=== allenap changed the topic of #launchpad-reviews to:  On call: allenap || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== rockstar changed the topic of #launchpad-reviews to:  On call: allenap, rockstar || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[16:07] <noodles775> sinzui: I'm happy to email the dev list regarding activity-streams etc., but I need to keep working on the next branch. Would you be ok if I re-order the comments and put the add-comment link at the bottom as henninge suggested, so that I'm not blocked on this?
[16:08] <sinzui> noodles775, yes
[16:08] <noodles775> Great, thanks.
[16:08] <EdwinGrubbs> leonardr: I'm looking at it now.
[16:08] <leonardr> k
[16:09] <henninge> noodles775, sinzui: I agree that this is an acceptable temporary solution. ;)
[16:09] <noodles775> Great, thanks henninge
[16:09] <sinzui> yes, I really think reverse listing for activity are what users want
[16:09] <EdwinGrubbs> leonardr: looks good. r=me
[16:09] <noodles775> Yes, me too.
[16:26]  * rockstar relocates to coffee shop
=== jcsackett changed the topic of #launchpad-reviews to:  On call: allenap, rockstar || Reviewing: - || queue: [jcsackett] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[16:29] <jcsackett> rockstar, allenap: https://code.edge.launchpad.net/~jcsackett/launchpad/remove-cache-private-650668/+merge/37143
[16:29] <jcsackett> i believe it's a two line diff.
=== Ursinha is now known as Ursinha-lunch
[16:30] <allenap> jcsackett: It says 566 :)
[16:30]  * jcsackett boggles.
[16:30] <jcsackett> allenap, that can't be right; let me see what i screwed up. :-P
[16:30] <noodles775> probably targetted to db-devel instead of devel (or vice-versa)?
[16:33] <jcsackett> allenap: i targeted the wrong branch. please take a look at this much smaller, much better MP: https://code.edge.launchpad.net/~jcsackett/launchpad/remove-cache-private-650668/+merge/37145
[16:33] <jcsackett> :-P
[16:34] <jcsackett> noodles775 was correct.
[16:34]  * noodles775 has done the same thing multiple times :)
[16:35] <jcsackett> honestly, i'm surprised i haven't done that before.
[16:35] <allenap> jcsackett: +1
[16:36] <jcsackett> thanks, allenap.
[16:56] <rockstar> allenap, is jcsackett still in the queue?
[16:56] <jcsackett> rockstar, nope, i'm not.
[16:56] <allenap> rockstar: Ah, no. Done.
=== rockstar changed the topic of #launchpad-reviews to:  On call: allenap, rockstar || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== Ursinha-lunch is now known as Ursinha
=== gary_poster is now known as gary-lunch
=== allenap changed the topic of #launchpad-reviews to:  On call: rockstar || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[17:34] <allenap> Cheerio rockstar, have a good OCR stint.
[17:34] <rockstar> allenap, thanks!
[17:34]  * rockstar takes the reins
=== salgado is now known as salgado-lunch
=== matsubara is now known as matsubara-lunch
[17:56] <james_w> rockstar: perhpas you would take a look at https://code.edge.launchpad.net/~james-w/bzr-builder/fix-text-of-nest-parts/+merge/37080 ?
[17:57] <james_w> it's an important bugfix for launchpad as well
[17:57] <rockstar> james_w, I'm amazed that no one took you up on this last night.
[17:57] <rockstar> Oh, nevermind.  It's a superseded merge.
[18:09] <rockstar> james_w, I can't set the mp status to Approved.
[18:09] <rockstar> Also, apparently I'm a community vote.
[18:10] <james_w> rockstar: I'll add you to ~bzr-builder-devs?
[18:10]  * rockstar joins team.
[18:11] <james_w> there we go
[18:11] <james_w> thanks for the review
=== gary-lunch is now known as gary_poster
[18:23] <james_w> rockstar: do you want a release to get these things in to LP?
[18:25] <rockstar> james_w, I think abentley is already working on that.
[18:26] <james_w> great
=== matsubara-lunch is now known as matsubara
[18:56]  * rockstar lunches
=== salgado-lunch is now known as salgado
[22:42] <benji> rockstar: I'm about to knock off, but want to throw https://code.edge.launchpad.net/~benji/lazr.restful/tweak-etag/+merge/37194 at you before I go.
=== benji changed the topic of #launchpad-reviews to: On call: rockstar || Reviewing: - || queue: [benji] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha