[11:45] <stub> https://code.edge.launchpad.net/~stub/launchpad/garbo/+merge/22680
[12:14] <stub> https://code.edge.launchpad.net/~stub/launchpad/garbo/+merge/22680
[12:14] <stub> jtv: ^^ that's a loop tuner patch if you are bored.
[12:15] <jtv> stub: oooh, cool.  I'm working on an oversized review, but I may just take this one inbetween as a light snack.
=== salgado changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[13:12] <jtv> stub: had some lucid bugs, but looking at your branch now.  _is_timed_out should be spelled _isTimedOut; and "extra" is not a great name... maybe extra_seconds or extension_seconds or somesuch?
[13:12] <jtv> (Though definitely nice to have that isolated)
[13:13] <jtv> Maybe invert the condition and call it _haveTimeLeft(seconds=0)?
[13:13] <jtv> ah no, that'd be the wrong way around wouldn't it
[13:13]  * jtv continues to flip-flop on that issue
[13:21] <stub> :)
=== salgado_ is now known as salgado
[13:29] <jtv> oh stub, there you are!  I just posted a comment.
[13:29] <stub> ta. Just eating dinner :)
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== abentley changed the topic of #launchpad-reviews to:  on call: abentley || reviewing: salgado || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:30] <abentley> salgado, what proposal do you want me to review?
[16:31] <salgado> abentley, remove-lp-service-openid
[16:32] <abentley> salgado, lp:~salgado/launchpad/remove-lp-services-openid is not proposed for merging.
[16:32] <salgado> I might have forgotten to create the mp
[16:32] <salgado> yeah, looks like I did
[16:34] <salgado> sorry abentley.  just proposed: https://code.edge.launchpad.net/~salgado/launchpad/remove-lp-services-openid/+merge/22807
[16:34] <salgado> it depends on my remove-auth-store branch, which removes canonical-identity-provider
[16:35] <abentley> salgado, cool.  I'll have a look in a few minutes when the diff is up.
[16:35] <salgado> thanks abentley
[16:37] <abentley> salgado, 1912 lines of diff!?
[16:37]  * salgado checks
[16:38] <salgado> abentley, the diff shown there includes what's in my remove-auth-store branch
[16:38] <salgado> any idea why it didn't take the pre-requisite branch into account when generating the diff?
[16:39] <abentley> salgado, no idea.
[16:39] <abentley> salgado, are both branches up-to-date?
[16:41] <salgado> yes, I think so.  let me try merging remove-auth-store again into the remove-lp-services one
[16:41] <abentley> salgado, I believe you've got a criss-cross merge, because you merged devel into each of them.
[16:42] <salgado> could be
[16:43] <salgado> abentley, any way to solve that so that the generated diff is correct?
[16:45] <abentley> salgado, if you merge remove-auth-store into remove-lp-services, it should be much better.
[16:45] <salgado> abentley, just did that
[16:47] <abentley> salgado, hmm.  So that brought it down to 943 lines.  Is correct now?
[16:47] <salgado> very likely, let me check
[16:47] <salgado> abentley, yes, that's correct.  it's just removals, though, as you can see
[16:48] <abentley> salgado, this is usually when I ask about preimplementation calls, but I assume it's been discussed to death.
[16:49] <abentley> salgado, r=me
[16:49] <salgado> thanks abentley
=== salgado is now known as salgado-brb
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-lunch
=== sinzui changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado-brb is now known as salgado
[18:26] <leonardr> abentley, looking for a quick review of https://code.edge.launchpad.net/~leonardr/launchpad/no-cookie-vary-header/+merge/22812
=== matsubara-lunch is now known as matsubara
[18:29] <abentley> leonardr, looking
[18:29] <adiroiban> abentley: hi, is there anything I need to do for having this branch landed https://code.edge.launchpad.net/~adiroiban/launchpad/bug-548999/+merge/22271 ?
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: leonardr || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[18:33] <abentley> leonardr, I believe there should be two blank lines, not one, between one heading and the next in xx-service.txt.  Otherwise, looks fine.
[18:36] <abentley> adiroiban, it needs to be run through the full test suite.
[18:37] <abentley> adiroiban, some other developers use ec2 to do this, but I don't use ec2.
[18:38] <abentley> adiroiban, for my own branches, I use my local boxes.
[18:39] <adiroiban> abentley: ok. I still have problems running the full test suite on my computer. Meanwhile, I will ask some other LP developer to send the branch to ec2. Thanks!
[18:39] <abentley> adiroiban, no problem.
[18:40] <abentley> sinzui, changing the topic does not get my attention, so I suggest asking me to perform the review instead of just changing the topic.
[18:40] <sinzui> abentley, I agree. I was told to get my children from school so I did not complete my task
=== abentley changed the topic of #launchpad-reviews to:  on call: abentley || reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[18:41] <abentley> sinzui, looking now.
[18:42] <leonardr> abentley: actually i'm pretty sure you don't have to put 2 blank lines anymore, now that we changed the format from =foo= to foo\n===
[18:43] <abentley> leonardr, the style guide uses "foo\n===" syntax, and says "Two blank lines are used to separate the start of a new section (a header). "
[18:44] <leonardr> ok
[18:44] <leonardr> abentley: i was going to write a launchpadlib branch with another test, and then i realized i can just put a launchpadlib test in launchpad now
[18:47] <leonardr> so i'll give you an incremental diff in a bit
[18:49] <abentley> sinzui, I think that "an unlisted package" may not convey "not packaged".
[18:49] <sinzui> abentley, I should hope NOT
[18:50] <sinzui> abentley, it is not related to the second button. It means the user has to find a package himself using +ubuntupkg
[18:50]  * sinzui think the UI review will be equally scathing
[18:50]  * sinzui needs to think how to say ":let me pick the ubuntu package myself"
[18:51] <abentley> sinzui, I see.  This is the page that is affected by the "not packaged" indicator, not the page where users select that?
[18:52] <sinzui> The not packages button does not use the radio button. It is a simple button that means stop showing me the form
[18:54] <sinzui> abentley, the problem is that this simple button will not fit in the the portlet. so we discussed moving the link to +ubuntupkg into the vocabulary. That was easy, but the intent is clearly ambiguous now.
[18:55] <abentley> sinzui, I am confused about why you are showing me a version with and without a selection.
[18:57] <abentley> s/selection/suggestion
[18:58] <abentley> sinzui, would it reasonable to present "not packaged in ubuntu" as a radio button?
[19:02] <abentley> sinzui, in lib/lp/registry/interfaces/product.py, I think you mean "project", not "projected", in "The projected can be rechecked after enough time "
[19:05] <abentley> sinzui, I don't know what to do about this, but it seems odd that if I select "This is not packaged in Ubuntu", in a year it will prompt me to specify what package it provides in Jaunty.
[19:08] <abentley> sinzui, ping me when you want to proceed.
=== abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== salgado_ is now known as salgado
=== matsubara is now known as matsubara-afk
=== jelmer is now known as Guest72846
=== jelmer_ is now known as jelmer
[21:23] <bac> sinzui: could you give a UI once over to https://code.edge.launchpad.net/~bac/launchpad/bug-524302/+merge/22180
[21:23] <bac> sinzui: i just pasted a screenshot into the last comment
[21:40]  * sinzui is already doing that
[21:42] <sidnei> i have a large branch needing review here, but im on holiday today. if anyone wants to take a peek, feel free, but no rush: https://code.edge.launchpad.net/~sidnei/lazr-js/yui-3.1.0/+merge/22825
=== salgado is now known as salgado-afk
[22:56] <james_w> abentley: thanks for the review. https://code.edge.launchpad.net/~james-w/launchpad/register-code-import/+merge/22668 is updated if you could have another look.
[23:06] <rockstar> james_w, I suspect abentley has EOD'd for the day.  :)
[23:07] <james_w> rockstar: me too, but I thought it was worth a try as the topic hadn't changed
[23:07] <james_w> but it's not important given that I'm not really here
[23:09] <rockstar> sidnei, are you around?
[23:16] <sidnei> rockstar, just happen to be
[23:16] <rockstar> sidnei, so, I'm confused a bit by your branch.  Why are we renaming classes, and what are all the other changes as well?
[23:16] <rockstar> It LOOKS like it's YUI 3.1 stuff.
[23:17] <sidnei> rockstar, yui 3.1.0 changed the prefix generated by ClassNameManager from yui- to yui3-
[23:17] <rockstar> sidnei, ah, okay.  So is this the branch that will make lazr-js support yui 3.1?
[23:17] <sidnei> rockstar, correct
[23:20] <sidnei> rockstar, the other changes are two kinds: 1) removing wait/resume where tests were failing, seems like it is not needed anymore in those cases and 2) double checking for null nodes
[23:20] <rockstar> sidnei, okay.
[23:20] <rockstar> sidnei, that helps.  I'll look over this in a bit, and you should have a review for you in the morning.
[23:20] <sidnei> rockstar, awesome. thanks!
[23:21] <sidnei> rockstar, if you get a chance, try the examples too. i noticed one or two misbehaving but didn't dig much yet. probably trivial to fix. the tests pass though.
[23:21] <rockstar> sidnei, okay.  Well, if they don't work, we probably need to figure out why before we land this.
[23:22] <sidnei> rockstar, one of them was the inline editor, when clicking on edit the static text isn't hidden. likely a css issue.