| === Ursinha-afk is now known as Ursinha | ||
| jtv | thumper: on what server are ITipChanged events fired and handled? | 03:49 |
|---|---|---|
| thumper | jtv: only within the branch scanner process | 03:50 |
| jtv | So that's where the buildfarmjobs all get created then, I guess. | 03:50 |
| jtv | What server does that run on? | 03:50 |
| jtv | Is that codehosting, or the script server? | 03:50 |
| jtv | thumper: (I need to know for a CP request) | 03:51 |
| jtv | thumper: I can guess it's codehosting, but this isn't a moment for guessing. :) | 03:55 |
| thumper | loganberry | 03:55 |
| jtv | So the script server! Thanks. | 03:56 |
| mwhudson | thumper: https://code.edge.launchpad.net/~mwhudson/launchpad/de-stupid-SpecificationDepCandidatesVocabulary/+merge/33728 | 05:05 |
| mwhudson | thumper, jtv: can you take a look at https://code.edge.launchpad.net/~mwhudson/launchpad/initialize-bzrlib-in-ec2-bug-624434/+merge/33730 ? | 05:39 |
| * thumper looks | 05:40 | |
| * jtv goes off to look | 05:40 | |
| thumper | why does it still say updating diff? | 05:41 |
| jtv | thumper: it's been doing that lately… for perhaps two hours at a time. | 05:41 |
| thumper | seems screwy | 05:41 |
| * thumper wonders if there is a bug filed | 05:42 | |
| jtv | mwhudson: is with still future? | 05:42 |
| thumper | jtv: it is for 2.5 | 05:42 |
| mwhudson | jtv: don't we still deploy on 2.5 ? | 05:42 |
| mwhudson | for this specific file i guess it doesn't matter | 05:42 |
| mwhudson | thumper: it doesn't say updating diff for me | 05:43 |
| jtv | We still have stuff on dapper afaik, so that makes sense | 05:43 |
| thumper | replication lag? | 05:43 |
| jtv | mwhudson: it's gone now | 05:43 |
| mwhudson | thumper: ta | 05:44 |
| jtv | thumper already approved it? | 05:44 |
| jtv | ah yes | 05:44 |
| jtv | mwhudson: under what circumstances does one need to initialize bzrlib? I'm wondering if we need to do the same in our buildfarm jobs. | 05:45 |
| mwhudson | jtv: we probably should, yeah | 05:45 |
| mwhudson | i don't think it's a specific requirement yet | 05:45 |
| jtv | yet? | 05:46 |
| mwhudson | well | 05:46 |
| mwhudson | i suspect as bzr changes, it will become more necessary | 05:46 |
| mwhudson | or less necessary, if the people with pitchforks win out i guess | 05:46 |
| jtv | heh | 05:49 |
| * jtv imagines a tough Hudson going "what kind of idiot brings a pitchfork to a flamewar" | 05:50 | |
| lifeless | well right now if you don't it won't log properly | 06:13 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | noodles775, got one for ya: https://code.edge.launchpad.net/~jtv/launchpad/recife-new-resetcurrenttranslation/+merge/33747 | 10:18 |
| jtv | I warn you, not for the faint of heart. | 10:18 |
| jtv | Not for the code per se; more for the murder mystery. | 10:18 |
| jtv | …and having thus ensured rapt attention from my potential reviewer, I trundle off to other work. | 10:19 |
| noodles775 | :) | 10:22 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | oh, hi benji! | 11:18 |
| jtv | did I mention you really did free us of our one daily-recurring oops? | 11:18 |
| benji | jtv: awesome! | 11:19 |
| jtv | very | 11:19 |
| benji | jelmer: for some reason ec2 land doesn't think you approved https://code.edge.launchpad.net/~benji/launchpad/check-in-wadl/+merge/33661: "ec2: ERROR: Merge proposal is not approved. Get it approved, or use --force to land it without approval." | 11:23 |
| benji | I'm about to go AFK, I suppose I'll use --force when I get back if you don't have a better idea. Thanks. | 11:23 |
| jelmer | benji: Sorry, I had voted approved but not marked the whole MP as approved. Done now. | 11:26 |
| noodles775 | jtv: on line 288 of the MP diff, why do you remove the assertion? | 12:03 |
| jtv | noodles775: not removing it—that's just an awkward slicing of the diff. Just replacing it with a clearer one: is_diverged. | 12:04 |
| jtv | That's a property that returns "potemplate is not None" | 12:04 |
| noodles775 | Right, thanks. | 12:05 |
| salgado | jtv, around? did you see my reply to those two reviews you did for me yesterday? | 12:27 |
| jtv | salgado: ah, I knew there was something I wanted to check up on… | 12:27 |
| jtv | salgado: ah, and of course I never saw the most important part in my email. | 12:28 |
| salgado | :) | 12:29 |
| jtv | That's why your later follow-up didn't make any sense to me. I never saw the part it was a follow-up to. | 12:30 |
| jtv | salgado: I _would_ be looking at your additional revisions at this point if codebrowse were willing to show them to me. | 12:30 |
| salgado | jtv, I can generate the incr. diffs here if you'd like | 12:31 |
| jtv | salgado: nah, at some point it'll start working again, I know it | 12:32 |
| jtv | See? I've already gone from "Please try again" to "This website is not available." | 12:32 |
| jtv | Er… hang on | 12:32 |
| jtv | salgado: got 'em. Some pretty big changes there; I won't be able to review them tonight. Will tomorrow do? | 12:51 |
| jelmer | noodles775: Can I add a trivial soyuz branch to the queue ? | 12:51 |
| salgado | jtv, big changes? which ones? I thought they were all small-ish | 12:52 |
| jelmer | StevenK: did you see my followup to the merge proposals you reviewed the other day? | 12:52 |
| jtv | salgado: isn't this one of your post-review changes? http://bazaar.launchpad.net/~salgado/launchpad/remove-security-upload-policy/revision/11427 | 12:52 |
| salgado | jtv, no, that's the initial diff you've reviewed | 12:53 |
| jtv | ah good | 12:53 |
| jtv | (before you say it: no, I did _not_ bother to read it :) | 12:54 |
| salgado | http://bazaar.launchpad.net/~salgado/launchpad/remove-security-upload-policy/revision/11428 | 12:54 |
| salgado | that's the one you haven't seen yet | 12:54 |
| salgado | jtv, 11427 is the mostly red diff, where I removed tons of stuff | 12:54 |
| jtv | salgado: 11428 is still giving me about 7 pages. Dinner just arrived; I'll look at it tomorrow. | 12:56 |
| jtv | The code, not the dinner. | 12:56 |
| salgado | jtv, do you mind if I get someone to review these other bits, then? | 12:57 |
| jtv | salgado: not at all. | 12:57 |
| salgado | cool, I'll do that, then | 12:57 |
| jtv | ok | 12:57 |
| === matsubara-afk is now known as matsubara | ||
| === henninge_ is now known as henninge | ||
| salgado | hi noodles775. care to do a couple follow up reviews for me? they're both quite simple | 13:14 |
| noodles775 | salgado: sure! | 13:18 |
| salgado | cool, thanks | 13:19 |
| === salgado changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: jtv || queue: [salgado*2] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: jelmer || queue: [salgado*2] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jelmer | noodles775: the MP is at https://code.edge.launchpad.net/~jelmer/launchpad/remove-archiveuploader-utils/+merge/33764 | 13:22 |
| noodles775 | jelmer: yep, just merged it (for some reason the diff not yet generated on the MP) | 13:23 |
| noodles775 | jelmer: r=me :) Nice and easy. | 13:24 |
| jelmer | noodles775: thanks :-) | 13:24 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: salgado || queue: [salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| salgado | noodles775, so, I think it's a good idea to take https://code.edge.launchpad.net/~salgado/launchpad/remove-security-upload-policy/+merge/33581 first | 13:26 |
| noodles775 | k, thanks. | 13:26 |
| leonardr | noodles775, i need a one-line review, can i sneak in before salgado's next review? | 13:26 |
| salgado | jtv's reviewed it, but there were test failures on the revision he reviewed | 13:26 |
| noodles775 | leonardr: sure, if it really is a one-liner ;) | 13:26 |
| leonardr | it is | 13:27 |
| leonardr | noodles, take a look at http://bazaar.launchpad.net/~leonardr/launchpad/optimized-length-2/revision/11392 which is my last reviewed code for this branch | 13:28 |
| leonardr | there's some code that sets resultset._select.* | 13:29 |
| leonardr | the tests failed because not all ResultSet objects have ._select, only the Storm ones | 13:29 |
| leonardr | so i added this line: | 13:30 |
| leonardr | if hasattr(resultset, '_select'): | 13:30 |
| noodles775 | salgado: So, looking at r11428 (the unreviewed revision), it adds TestStagedBinaryUploadBase back in, but not TestStagedSecurityUploads? | 13:32 |
| salgado | noodles775, right, I've only added it back in because test_buildduploads.py needs TestStagedBinaryUploadBase | 13:33 |
| gmb | allenap, Can I get an r[s]= from you to merge that branch from yesterday into devel. I merged it into db-devel by mistake. | 13:33 |
| salgado | noodles775, I've removed TestStagedSecurityUploads because that's a test for the 'security' upload policy, which the branch removed | 13:33 |
| noodles775 | OK. | 13:33 |
| * noodles775 reads the MP | 13:34 | |
| gmb | allenap, Actually, I suspect I don't need to ask you for *another* r=, since the code's the same :) | 13:34 |
| salgado | noodles775, and I've added a new unit test (in the other MP) to exercise the same code that was exercised by the doctest chunk I've removed | 13:40 |
| noodles775 | salgado: The doctest snippet that you removed in r11428 (reject instead of fail for non-existent pocket), you mention that you'll cover it with a unit test in the following branch... | 13:40 |
| noodles775 | lol | 13:40 |
| noodles775 | but | 13:40 |
| salgado | :) | 13:40 |
| salgado | http://bazaar.launchpad.net/~salgado/launchpad/vostok-upload-policy/revision/11432 | 13:41 |
| noodles775 | as far as I can see, the following branch tests that an assertion is raised, but not that a rejection is sent | 13:41 |
| * noodles775 double checks, guessing that he missed it. | 13:41 | |
| salgado | that's correct | 13:41 |
| noodles775 | Is there a test elsewhere that ensures when the assertion is raised an email is sent? | 13:42 |
| salgado | I thought just checking for the exception was good enough, as we have plenty of tests showing a rejection is sent when there's an exception | 13:42 |
| salgado | I could be wrong | 13:42 |
| noodles775 | OK. | 13:42 |
| jml | noodles775, would you please review https://code.edge.launchpad.net/~jml/launchpad/one-email-on-crash/+merge/33768 ? | 13:44 |
| jml | there appears to be some glitch on production stopping diffs from being generated. | 13:44 |
| jml | I guess I should escalate to a losa | 13:44 |
| noodles775 | jml: sure. | 13:44 |
| jml | noodles775, thanks. | 13:45 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: leonardr || queue: [salgado, jml] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| noodles775 | leonardr: I don't see a one-liner? It's r11442 that needs to be reviewed right? This: http://bazaar.launchpad.net/~leonardr/launchpad/optimized-length-2/revision/11442 | 13:51 |
| jml | yay, emails. | 13:55 |
| leonardr | noodles775, 11442 corrects a typo made to the original one-liner. i was hoping to just give you the code so it wouldn't be so complicated | 13:59 |
| leonardr | i'll just do another merge proposal and get in the queue | 13:59 |
| noodles775 | leonardr: I'm happy to review just the one-line that's changed, but currently I can only see it in 11442 along with a few other changes. If the rest is already reviewed, just paste a diff and I can review that, otherwise yes, a new MP is probably easier. | 14:01 |
| leonardr | noodles775: i see the problem. i had to create a new branch with the old changes, so all the old changes show up as one revision | 14:04 |
| leonardr | this code is the only code that hasn't been reviewed yet: | 14:04 |
| leonardr | + if hasattr(resultset, '_select'): | 14:04 |
| leonardr | + resultset._select.limit = Undef | 14:04 |
| leonardr | + resultset._select.offset = Undef | 14:04 |
| noodles775 | leonardr: r=me (I assume the second two lines were there previously, just not indented). Thanks! | 14:05 |
| leonardr | noodles, yeah | 14:05 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: jml || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| noodles775 | jml: why do you set test_directory when it's passed as None? | 14:51 |
| noodles775 | (that is, why do you set it to a non-existent directory) | 14:52 |
| jml | noodles775, umm, not sure. maybe because stuff explodes. | 14:52 |
| * jml tries. | 14:52 | |
| noodles775 | According to the docs, I'd assume it wouldn't explode when its None, but will in its current state? | 14:53 |
| noodles775 | But yes, science is the best answer :) | 14:53 |
| salgado | thanks for the reviews, noodles775 | 14:54 |
| noodles775 | jml: ah, you just moved that code... I'd not realised. But still worth checking. | 14:55 |
| noodles775 | salgado: np. | 14:55 |
| jml | noodles775, yeah, deleting that bit works. | 14:56 |
| noodles775 | jml: OK, the other option would be to make it a required arg (which I guess was the original intent?) | 14:57 |
| jml | noodles775, well, it's a required arg of the constructor, but most tests just don't care about it. | 15:02 |
| noodles775 | k | 15:02 |
| jml | noodles775, so I'd rather leave it optional in the make_foo() method | 15:02 |
| noodles775 | Great. | 15:02 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === salgado is now known as salgado-lunch | ||
| === Ursinha is now known as Ursinha-lunch | ||
| === deryck is now known as deryck[lunch] | ||
| === benji is now known as benji-lunch | ||
| === salgado-lunch is now known as salgado | ||
| === matsubara is now known as matsubara-lunch | ||
| === Ursinha-lunch is now known as Ursinha | ||
| === benji-lunch is now known as benji | ||
| === deryck[lunch] is now known as deryck | ||
| === matsubara-lunch is now known as matsubara | ||
| leonardr | salgado, i have a branch at https://code.launchpad.net/~leonardr/lazr.restful/its-really-read-only/+merge/33820 that you might enjoy reviewing | 19:54 |
| leonardr | (or anyone, really) | 19:55 |
| salgado | leonardr, sure, I'll take it | 19:57 |
| leonardr | salgado, thanks | 19:58 |
| deryck | sinzui, could I get your review (code/ui) of my CSS changes? | 19:59 |
| sinzui | yep | 19:59 |
| deryck | sinzui, thanks! https://code.edge.launchpad.net/~deryck/launchpad/good-god-comment-fonts-yall/+merge/33822 | 20:00 |
| sinzui | deryck, do you know what uses p.search-results? | 20:03 |
| deryck | sinzui, looked to be things like suggestions of projects when registering a new project. I saw no difference when testing locally, so saw no harm in making all these 116% changes together. | 20:04 |
| sinzui | I'll look into that. that implies something else redefined the style. maybe we do not need it | 20:05 |
| salgado | tagged_as_readonly = ('readonly' in tags and tags.get('readonly')) | 20:10 |
| salgado | if (readonly and not tagged_as_readonly): | 20:10 |
| salgado | raise TypeError(...) | 20:10 |
| salgado | if not readonly and tagged_as_readonly: | 20:10 |
| salgado | readonly = True | 20:10 |
| salgado | leonardr, how about that? | 20:11 |
| salgado | that won't work, I think | 20:12 |
| salgado | yeah, it will raise TypeError if the field is readonly and there's no readonly tag | 20:13 |
| sinzui | deryck, I think I see YUI font css on the body tag. I see our font rules are work, but YUI is also defining font-size and line-height | 20:16 |
| sinzui | I hate YUI | 20:16 |
| deryck | yeah, it made this work much harder than it should have been, IMHO | 20:16 |
| deryck | and I generally love YUI | 20:16 |
| sinzui | yep, the lines spacing on bug descriptions is easy to read when I disable the YUI rule | 20:17 |
| deryck | I noticed in this work the great mis-match in descriptions and the rest of the page | 20:18 |
| leonardr | salgado, fine | 20:18 |
| sinzui | but why is it's rule *after* ours? | 20:18 |
| salgado | leonardr, that won't work, but I have another idea I'm suggesting in my review | 20:18 |
| leonardr | salgado: i can just change the names around so i don't redefine 'readonly' | 20:18 |
| deryck | sinzui, you mean the description's rule? I assumed it was set via JS on the fly. But I didn't look into it. | 20:20 |
| sinzui | no, webkit provides an inspector that shows the order of rules, and it lets you toggle them | 20:20 |
| sinzui | I thought it was the widget too, but it only applies margin rules | 20:21 |
| deryck | well, there are the rules in the stylesheets in lazr-js. That's what I meant. | 20:23 |
| deryck | well, part of what I meant/thought. Not saying it correctly. :-) | 20:23 |
| deryck | I'm hesitant to change the CSS on the description widget for this branch, just because it's quite fiddling to get all the spacing and alignment correct. | 20:24 |
| jcsackett | EdwinGrubbs: did you/will you have a chance to look at the update to my MP? | 20:54 |
| EdwinGrubbs | jcsackett: I actually just started to look at it. | 20:56 |
| jcsackett | EdwinGrubbs: fantastic. :-) | 20:56 |
| EdwinGrubbs | jcsackett: I commented on one more thing that I want changed. I marked it Approved, so you don't have to reply on the MP after you make the changes. | 21:18 |
| jcsackett | EdwinGrubbs: sounds good. thanks, man. | 21:20 |
| === matsubara is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!