| stub | https://code.edge.launchpad.net/~stub/launchpad/garbo/+merge/22680 | 11:45 |
|---|---|---|
| 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:14 |
| jtv | stub: oooh, cool. I'm working on an oversized review, but I may just take this one inbetween as a light snack. | 12:15 |
| === 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 | ||
| 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:12 |
| 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:13 | |
| stub | :) | 13:21 |
| === salgado_ is now known as salgado | ||
| jtv | oh stub, there you are! I just posted a comment. | 13:29 |
| stub | ta. Just eating dinner :) | 13:29 |
| === 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 | ||
| abentley | salgado, what proposal do you want me to review? | 16:30 |
| salgado | abentley, remove-lp-service-openid | 16:31 |
| 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:32 |
| 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:34 |
| abentley | salgado, cool. I'll have a look in a few minutes when the diff is up. | 16:35 |
| salgado | thanks abentley | 16:35 |
| abentley | salgado, 1912 lines of diff!? | 16:37 |
| * salgado checks | 16:37 | |
| 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:38 |
| abentley | salgado, no idea. | 16:39 |
| abentley | salgado, are both branches up-to-date? | 16:39 |
| 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:41 |
| salgado | could be | 16:42 |
| salgado | abentley, any way to solve that so that the generated diff is correct? | 16:43 |
| 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:45 |
| 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:47 |
| abentley | salgado, this is usually when I ask about preimplementation calls, but I assume it's been discussed to death. | 16:48 |
| abentley | salgado, r=me | 16:49 |
| salgado | thanks abentley | 16:49 |
| === 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 | ||
| leonardr | abentley, looking for a quick review of https://code.edge.launchpad.net/~leonardr/launchpad/no-cookie-vary-header/+merge/22812 | 18:26 |
| === matsubara-lunch is now known as matsubara | ||
| 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 ? | 18:29 |
| === 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 | ||
| 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:33 |
| abentley | adiroiban, it needs to be run through the full test suite. | 18:36 |
| abentley | adiroiban, some other developers use ec2 to do this, but I don't use ec2. | 18:37 |
| abentley | adiroiban, for my own branches, I use my local boxes. | 18:38 |
| 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:39 |
| 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 | 18:40 |
| === 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 | ||
| abentley | sinzui, looking now. | 18:41 |
| 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:42 |
| 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:43 |
| 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:44 |
| leonardr | so i'll give you an incremental diff in a bit | 18:47 |
| abentley | sinzui, I think that "an unlisted package" may not convey "not packaged". | 18:49 |
| sinzui | abentley, I should hope NOT | 18:49 |
| 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:50 | |
| abentley | sinzui, I see. This is the page that is affected by the "not packaged" indicator, not the page where users select that? | 18:51 |
| sinzui | The not packages button does not use the radio button. It is a simple button that means stop showing me the form | 18:52 |
| 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:54 |
| abentley | sinzui, I am confused about why you are showing me a version with and without a selection. | 18:55 |
| abentley | s/selection/suggestion | 18:57 |
| abentley | sinzui, would it reasonable to present "not packaged in ubuntu" as a radio button? | 18:58 |
| 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:02 |
| 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:05 |
| abentley | sinzui, ping me when you want to proceed. | 19:08 |
| === 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 | ||
| 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:23 |
| * sinzui is already doing that | 21:40 | |
| 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 | 21:42 |
| === salgado is now known as salgado-afk | ||
| 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. | 22:56 |
| rockstar | james_w, I suspect abentley has EOD'd for the day. :) | 23:06 |
| 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:07 |
| rockstar | sidnei, are you around? | 23:09 |
| 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:16 |
| 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:17 |
| 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:20 |
| 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:21 |
| sidnei | rockstar, one of them was the inline editor, when clicking on edit the static text isn't hidden. likely a css issue. | 23:22 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!