| thumper | mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/get-branch-by-date/+merge/18816 | 01:44 |
|---|---|---|
| thumper | it is pretty simple | 01:44 |
| mwhudson | thumper: continuing your lounge tour? | 01:44 |
| thumper | mwhudson: yeah | 01:44 |
| thumper | in CHC now | 01:45 |
| thumper | I have about an hour before the next boarding call | 01:45 |
| === henninge changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: - || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === jpds changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: - || queue [jpds(518232)] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | jpds: Hi! I only noticed your queue entry by chance. You should always poke me (the OCR) when you want a review ... ;-) | 10:44 |
| henninge | Unfortunately, I will be afk for a little while now, but I'll look at it afterwards. | 10:45 |
| === henninge is now known as heninge-afk | ||
| noodles775 | \w 13 | 10:56 |
| === matsubara-afk is now known as matsubara | ||
| jpds | heninge-afk: Sorry about that. | 11:23 |
| === heninge-afk is now known as heninge | ||
| === heninge is now known as henninge | ||
| jpds | noodles775: Hi, can I have a UI review: https://code.launchpad.net/~jpds/launchpad/fix_518385/+merge/18812 | 11:30 |
| noodles775 | Hi jpds, afaik, sinzui is keen to build up UI reviews (assuming we're still going to do some sort of UI review process ourselves). | 11:35 |
| noodles775 | jpds: so please request one from him on the MP... I'll take a look at it after that. | 11:35 |
| jpds | noodles775: Will do. | 11:36 |
| henninge | jpds: No need to be sorry, I am looking at it now. | 11:41 |
| === henninge changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: jpds(518232) || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jpds | henninge: That's another MP. :) | 11:43 |
| henninge | jpds: I know, I was referring to your earlier "sorry about that" ;-) | 11:44 |
| henninge | jpds: I am looking at this now: https://code.edge.launchpad.net/~jpds/launchpad/fix_518232/+merge/18778 | 11:45 |
| jpds | henninge: Cool. | 11:48 |
| noodles775 | hi henning, when you're free, can you review the following? https://code.edge.launchpad.net/~michael.nelson/launchpad/494391-ugly-upload-error-message/+merge/18831 | 12:23 |
| henninge | noodles775: sure, just stick it in the queue | 12:25 |
| === noodles775 changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: jpds(518232) || queue [noodles775] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | jpds: I don't see any pre-imp information in your MP. | 12:36 |
| henninge | jpds: Who from the registry team did you talk to about this? | 12:36 |
| jpds | henninge: sinzui in millbank. | 12:37 |
| henninge | jpds: face-to-face, sweet ;) | 12:37 |
| henninge | jpds: I find the test a bit hard to read. Couldn't it be made to focus on what is being restricted? | 12:43 |
| henninge | jpds: by adding a few more "...", for example? | 12:44 |
| jpds | henninge: Could do that. | 12:45 |
| henninge | jpds: at least on the repeats. | 12:45 |
| henninge | jpds: cool, that's all I could find to complain about ... ;-) r=me | 12:48 |
| jpds | henninge: Something like http://pastebin.ubuntu.com/371650/ ? | 12:50 |
| henninge | jpds: yes, that looks good ;-) | 12:51 |
| jpds | It doesn't seem to like it: http://pastebin.ubuntu.com/371653/ | 12:53 |
| henninge | jpds: you have consecutive "..." in there. Sorry, didn't look close enough. | 12:56 |
| jpds | henninge: On which line? | 12:57 |
| henninge | jpds: there is a new line and white space in between but still ... | 12:57 |
| henninge | jpds: oh, also I think leading "..." aren't allowed either. | 12:58 |
| henninge | jpds: or maybe that is the only problem, and consecutive ones are allowed. | 12:59 |
| henninge | Try it out ... ;-) | 12:59 |
| henninge | jpds: maybe like this? http://pastebin.ubuntu.com/371658/ | 13:01 |
| salgado | jml, around? I'd like to know what you think of https://code.launchpad.net/~salgado/launchpad/kill-rogue-ec2-instances/+merge/18834 | 13:01 |
| henninge | jpds: oh no, like this I meant ... ;) http://pastebin.ubuntu.com/371659/ | 13:02 |
| jml | salgado, I'll take a look | 13:08 |
| jpds | henninge: Pushed changes to test and good to go. :) | 13:10 |
| henninge | jpds: ok, so it was the leading "..." ... ;-) Thanks for trying that out. | 13:14 |
| jml | salgado, done | 13:17 |
| henninge | noodles775: review sent. | 13:18 |
| === henninge changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: lunch || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| salgado | jml, thanks, but I just realized prepare_tests() is also called when setting up a demo instance, so I'll have to either more the new code to a different method which is only called when running the test suite or use a conditional | 13:19 |
| jml | salgado, ahh ok | 13:19 |
| jml | salgado, let me have a look | 13:19 |
| salgado | run_tests(), maybe? | 13:20 |
| jml | salgado, that'd work. I was thinking maybe giving EC2TestRunner's constructor a timeout parameter, and passing that in from builtins.py | 13:22 |
| salgado | yeah, that'd be nicer | 13:23 |
| jml | salgado, the EC2TestRunner class needs to be exploded. | 13:39 |
| noodles775 | Thanks henninge | 13:47 |
| salgado | jml, check out the updated diff in the mp. this one doesn't schedule a shutdown on demo instances | 13:49 |
| jml | salgado, looking... | 13:49 |
| jml | salgado, Looks good. I would have picked seconds, rather than minutes. | 13:51 |
| salgado | jml, do you think we need that much granularity? | 13:51 |
| jml | salgado, no, I just am unused to seeing computers talk about minutes ): | 13:52 |
| jml | I meant to say ":)" | 13:52 |
| jml | salgado, no big deal | 13:52 |
| salgado | cool. :) | 13:52 |
| === henninge_ is now known as henninge | ||
| jpds | henninge: Could you ec2 / land the branch for me please. | 14:25 |
| henninge | jpds: yes sure. Did set a commit message? | 14:26 |
| henninge | *you* ;) | 14:26 |
| jpds | henninge: Yep. | 14:26 |
| henninge | jpds: cool, will land it now. | 14:26 |
| === matsubara is now known as matsubara-lunch | ||
| === salgado is now known as salgado-lunch | ||
| bac | henninge: can i add a review to the queue? | 15:06 |
| bac | https://code.launchpad.net/~bac/launchpad/bug-514447-anon-api/+merge/18842 | 15:06 |
| * bac assumes 'yes' | 15:06 | |
| === bac changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: lunch || queue [bac] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | bac: please do | 15:06 |
| bac | thanks | 15:06 |
| henninge | oh, done ;-) | 15:06 |
| henninge | np, my job here ... ;) | 15:07 |
| allenap | henninge: Fancy another one? | 15:09 |
| allenap | https://code.edge.launchpad.net/~allenap/launchpad/twisted-threading-bug-491870/+merge/18843 | 15:09 |
| === allenap changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: lunch || queue [bac, allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | allenap: sure | 15:10 |
| allenap | henninge: Thanks :) | 15:10 |
| === abentley changed the topic of #launchpad-reviews to: on-call: henninge, abentley || reviewing: lunch, bac || queue [allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | abentley: Hi! ;) | 15:24 |
| === henninge changed the topic of #launchpad-reviews to: on-call: henninge, abentley || reviewing: allenap, bac || queue [allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| henninge | glad I looked here again before starting on bac's branch ... ;) | 15:25 |
| henninge | abentley: Hi! ;) | 15:25 |
| henninge | glad I looked here again before starting on bac's branch ... ;) | 15:25 |
| abentley | henninge, hi. | 15:26 |
| abentley | bac, your branch looks reasonable. Did you actually make any changes to set_attributes, or is it just formatting? | 15:27 |
| bac | abentley: formatting. that section was all on one super-long line | 15:27 |
| abentley | bac, cool. I think the second line needs one more space (before translation_focus). | 15:28 |
| bac | ok | 15:28 |
| abentley | bac, also, should that be alphabetically sorted? | 15:28 |
| bac | abentley: i don't know that sorting is a requirement. i didn't pay any attention to the content, just did the reformatting. | 15:29 |
| abentley | bac, okay. | 15:30 |
| abentley | bac, r=me. | 15:30 |
| bac | great, thanks | 15:30 |
| abentley | bac, another way of handling the AnonymousAuthorization issue would be to merge jamal's branch into yours and then set it as a prerequisite branch. | 15:31 |
| bac | abentley: yeah, i realized i should've done that. | 15:32 |
| abentley | bac, just thought I should mention because prerequisite branches are newish. | 15:33 |
| === abentley changed the topic of #launchpad-reviews to: henninge, abentley || reviewing: allenap, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-lunch is now known as matsubara | ||
| === salgado-lunch is now known as salgado | ||
| jamalta | you can define prerequiesite branches now? that's pretty cool | 16:02 |
| bac | abentley: actually, could you explain how to create pre-req branch relationship? | 16:03 |
| abentley | bac, when you are working on branch FOO, but that needs changes from branch BAR, you either branch FOO from BAR or merge BAR into FOO. When you create the merge proposal, you specify that BAR is the prerequisite branch. | 16:05 |
| bac | abentley: can you do that last step using 'bzr send'? | 16:06 |
| abentley | bac, no. | 16:06 |
| abentley | bac, you can do it with lp-submit (from pipeline), though. | 16:06 |
| bac | abentley: can you do that even if not using pipelines? if so, is lp-submit replacing send in general? | 16:07 |
| abentley | bac, lp-submit doesn't require pipeslines, but the prerequisite is currently hardwired to be the previous pipe. | 16:08 |
| henninge | allenap: I thought lambda func were nothedl in high regard inLP code? | 16:08 |
| abentley | bac, I intend for lp-submit to replace send, and I have a branch that adds it to bzr itself. | 16:09 |
| bac | abentley: ok. | 16:09 |
| allenap | henninge: In Twisted it's a very common pattern. I don't think I'll get shot for that. | 16:12 |
| henninge | allenap: I am the one with the gun here, though ... :-P | 16:13 |
| allenap | henninge: Heh :) | 16:13 |
| henninge | ;) | 16:13 |
| henninge | allenap: I have to admit, I lack Twisted familiarity here ... | 16:14 |
| allenap | henninge: There are so many callbacks that to write a def for each one gets old very quickly. Using lambdas instead is, I think, accepted practice. Even with the `lambda ignore: ...` pattern. | 16:15 |
| allenap | henninge: Actually, I can probably change it to use functools.partial. Let me see. | 16:17 |
| allenap | henninge: Ah, no I can't. | 16:17 |
| henninge | allenap: I am not too happy with letting another projects coding styles leak into ours. | 16:18 |
| henninge | allenap: I think the real question is if we are/will be using Twisted a lot or if this is an isolated case. | 16:18 |
| henninge | allenap: I'd like a second opinion on this. | 16:20 |
| henninge | abentley: do you have a minute? | 16:20 |
| abentley | henninge, sure. | 16:20 |
| allenap | henninge: The code team use Twisted quite a lot and I see several uses of it in lp.code and lp.codehosting. | 16:21 |
| henninge | abentley: we are discussing the use of lambda, which our coding styles forbid but seem to be in heavy use with Twisted. | 16:21 |
| allenap | henninge: Actually, it's probably not worth arguing about this too much. I'm happy to put the calls into a def and be done with it. That way we're both happy :) | 16:22 |
| abentley | henninge, I wasn't aware that our coding styles forbid it. | 16:22 |
| henninge | abentley: https://dev.launchpad.net/PythonStyleGuide#Use%20of%20lambda,%20and%20operator.attrgetter | 16:22 |
| henninge | allenap: sorry to be PITA about it but the use of lambdas goes against explicit coding styles, so I like make sure to DRT. | 16:23 |
| henninge | DTRT | 16:24 |
| henninge | :-) | 16:24 |
| henninge | allenap: yes, with that I'd be happy :) | 16:24 |
| abentley | henninge, I think when coding style makes writing code painful, it should be abandoned. | 16:24 |
| henninge | abentley: I agree, that's why I raised the question if this is just an isolated case or if it happens regularly. | 16:25 |
| henninge | If it's just a single case, the extra pain is neglectable, otherwise we should adapt our coding styles. | 16:26 |
| abentley | henninge, with Twisted, you have to define many more callables than with normal python. | 16:26 |
| abentley | henninge, often these callables merely adapt existing functions for Twisted use. | 16:27 |
| henninge | abentley, allenap: I will put the request to update the coding styles for Twisted code on the reviewers' meeting agenda. | 16:28 |
| allenap | henninge: Cool. | 16:28 |
| abentley | henninge, so lambdas are pretty useful when writing Twisted code. | 16:28 |
| henninge | allenap: leave it as it is for now. | 16:28 |
| allenap | henninge: Okay. | 16:28 |
| henninge | abentley: I see. | 16:28 |
| abentley | henninge, also, the purpose of the callables is to act as callbacks, not to support code reuse. | 16:29 |
| henninge | abentley: that is a good point to support this exception. | 16:29 |
| abentley | henninge, so it's really a different scenario. | 16:29 |
| henninge | allenap: sorry, got a bit distracted ... | 16:57 |
| allenap | henninge: :) | 16:57 |
| henninge | allenap: Why did you definen a parameter "install_signal_handlers"? | 16:57 |
| allenap | henninge: Otherwise there's an error in, I think, checkwatches-cli-switches.txt for a layering violation; SIGCHLD handler left behind. | 16:58 |
| henninge | allenap: I mean, why is it configurable? | 16:58 |
| henninge | allenap: so, are you saying setting it to "False" would produce an error? | 16:59 |
| allenap | henninge: Just to make it work for testing. I could, for example, pass a reactor it, or make the reactor a class var and subclass. Maybe the latter would be better. | 16:59 |
| henninge | allenap: argh, I missed that in the test. | 17:00 |
| allenap | henninge: If it were left as True in externalbugtracker.txt (not checkwatches-cli-switches.txt; I got that wrong) then there would be an error. The test would pass, but the test suite would complain. | 17:01 |
| henninge | I see. I don't know enough about Twisted to asses the other options you mentioned. | 17:01 |
| henninge | allenap: I was just wondering that only one code path was tested but now I understand it is to make it testable. | 17:02 |
| henninge | allenap: Please make a note in the doc string for install_signal_handlers. | 17:02 |
| henninge | for __init__, of course | 17:02 |
| allenap | henninge: Sure. I think I might change it to a subclass, but I'll add some more docs to make it clear. | 17:03 |
| henninge | thanks | 17:03 |
| henninge | allenap: I wonder if there should be a test for SerialScheduler in some way. | 17:07 |
| allenap | henninge: I guess it's implicitly tested, but I'll add an explicit test too. | 17:25 |
| * allenap goes to look after ill kids. | 17:36 | |
| henninge | allenap: review sent | 17:38 |
| === henninge changed the topic of #launchpad-reviews to: abentley || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| * henninge bows out. | 17:39 | |
| gmb | abentley: Hi. Do you have time to review https://code.edge.launchpad.net/~gmb/launchpad/fix-b.l.n-timeouts-bug-517798/+merge/18866 for me? | 18:54 |
| gmb | Simple fix for bug 517798 | 18:54 |
| mup | Bug #517798: Launchpad beta timing out three times when I try to access the bug page. <oops> <timeout> <Launchpad Bugs:In Progress by gmb> <https://launchpad.net/bugs/517798> | 18:54 |
| leonardr | rockstar, version branch coming your way | 18:59 |
| rockstar | leonardr, cool. | 18:59 |
| abentley | gmb, happy to. | 19:03 |
| === abentley changed the topic of #launchpad-reviews to: abentley || reviewing: gmb || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| abentley | gmb: r-me | 19:05 |
| gmb | abentley: Excellent, thank you. | 19:09 |
| leonardr | rockstar: https://code.edge.launchpad.net/~leonardr/lazr.restful/multiversion-pagetest/+merge/18868 | 19:15 |
| rockstar | leonardr, great. I'm assisting in a cherry pick right now, will review when that's done. | 19:16 |
| === abentley changed the topic of #launchpad-reviews to: abentley || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-afk | ||
| rockstar | leonardr, hi. Something is fishy about this diff. | 20:05 |
| rockstar | leonardr, is it not based on one of the previous branches I reviewed? | 20:05 |
| rockstar | leonardr, nevermind. I plead temporary insanity. | 20:06 |
| === salgado is now known as salgado-afk | ||
| rockstar | leonardr, so, if there's no mutator specified for 2.0, it uses the mutator specified from 1.0? | 20:13 |
| leonardr | rockstar: right | 20:26 |
| leonardr | same as other named operations | 20:27 |
| rockstar | leonardr, so is "beta" the most recent version, and then 3.0, 2.0, and 1.0? | 20:28 |
| leonardr | rockstar: right, according to the list of versions defined in the iwebserviceconfiguration | 20:28 |
| === abentley changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === jamalta is now known as jamalta-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!