[03:49] <jtv> thumper: on what server are ITipChanged events fired and handled?
[03:50] <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:51] <jtv> thumper: (I need to know for a CP request)
[03:55] <jtv> thumper: I can guess it's codehosting, but this isn't a moment for guessing.  :)
[03:55] <thumper> loganberry
[03:56] <jtv> So the script server!  Thanks.
[05:05] <mwhudson> thumper: https://code.edge.launchpad.net/~mwhudson/launchpad/de-stupid-SpecificationDepCandidatesVocabulary/+merge/33728
[05:39] <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:40]  * thumper looks
[05:40]  * jtv goes off to look
[05:41] <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:42]  * 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:43] <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:44] <mwhudson> thumper: ta
[05:44] <jtv> thumper already approved it?
[05:44] <jtv> ah yes
[05:45] <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:46] <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:49] <jtv> heh
[05:50]  * jtv imagines a tough Hudson going "what kind of idiot brings a pitchfork to a flamewar"
[06:13] <lifeless> well right now if you don't it won't log properly
[10:18] <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:19] <jtv> …and having thus ensured rapt attention from my potential reviewer, I trundle off to other work.
[10:22] <noodles775> :)
[11:18] <jtv> oh, hi benji!
[11:18] <jtv> did I mention you really did free us of our one daily-recurring oops?
[11:19] <benji> jtv: awesome!
[11:19] <jtv> very
[11:23] <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:26] <jelmer> benji: Sorry, I had voted approved but not marked the whole MP as approved. Done now.
[12:03] <noodles775> jtv: on line 288 of the MP diff, why do you remove the assertion?
[12:04] <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:05] <noodles775> Right, thanks.
[12:27] <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:28] <jtv> salgado: ah, and of course I never saw the most important part in my email.
[12:29] <salgado> :)
[12:30] <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:31] <salgado> jtv, I can generate the incr. diffs here if you'd like
[12:32] <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:51] <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:52] <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:53] <salgado> jtv, no, that's the initial diff you've reviewed
[12:53] <jtv> ah good
[12:54] <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:56] <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:57] <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
[13:14] <salgado> hi noodles775.  care to do a couple follow up reviews for me?  they're both quite simple
[13:18] <noodles775> salgado: sure!
[13:19] <salgado> cool, thanks
[13:22] <jelmer> noodles775: the MP is at https://code.edge.launchpad.net/~jelmer/launchpad/remove-archiveuploader-utils/+merge/33764
[13:23] <noodles775> jelmer: yep, just merged it (for some reason the diff not yet generated on the MP)
[13:24] <noodles775> jelmer: r=me :) Nice and easy.
[13:24] <jelmer> noodles775: thanks :-)
[13:26] <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:27] <leonardr> it is
[13:28] <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:29] <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:30] <leonardr> so i added this line:
[13:30] <leonardr> if hasattr(resultset, '_select'):
[13:32] <noodles775> salgado: So, looking at r11428 (the unreviewed revision), it adds TestStagedBinaryUploadBase back in, but not TestStagedSecurityUploads?
[13:33] <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:34]  * 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:40] <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:41] <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:42] <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:44] <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:45] <jml> noodles775, thanks.
[13:51] <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:55] <jml> yay, emails.
[13:59] <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
[14:01] <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:04] <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:05] <noodles775> leonardr: r=me (I assume the second two lines were there previously, just not indented). Thanks!
[14:05] <leonardr> noodles, yeah
[14:51] <noodles775> jml: why do you set test_directory when it's passed as None?
[14:52] <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:53] <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:54] <salgado> thanks for the reviews, noodles775
[14:55] <noodles775> jml: ah, you just moved that code... I'd not realised. But still worth checking.
[14:55] <noodles775> salgado: np.
[14:56] <jml> noodles775, yeah, deleting that bit works.
[14:57] <noodles775> jml: OK, the other option would be to make it a required arg (which I guess was the original intent?)
[15:02] <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.
[19:54] <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:55] <leonardr> (or anyone, really)
[19:57] <salgado> leonardr, sure, I'll take it
[19:58] <leonardr> salgado, thanks
[19:59] <deryck> sinzui, could I get your review (code/ui) of my CSS changes?
[19:59] <sinzui> yep
[20:00] <deryck> sinzui, thanks!  https://code.edge.launchpad.net/~deryck/launchpad/good-god-comment-fonts-yall/+merge/33822
[20:03] <sinzui> deryck, do you know what uses p.search-results?
[20:04] <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:05] <sinzui> I'll look into that. that implies something else redefined the style. maybe we do not need it
[20:10] <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:11] <salgado> leonardr, how about that?
[20:12] <salgado> that won't work, I think
[20:13] <salgado> yeah, it will raise TypeError if the field is readonly and there's no readonly tag
[20:16] <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:17] <sinzui> yep, the lines spacing on bug descriptions is easy to read when I disable the YUI rule
[20:18] <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:20] <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:21] <sinzui> I thought it was the widget too, but it only applies margin rules
[20:23] <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:24] <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:54] <jcsackett> EdwinGrubbs: did you/will you have a chance to look at the update to my MP?
[20:56] <EdwinGrubbs> jcsackett: I actually just started to look at it.
[20:56] <jcsackett> EdwinGrubbs: fantastic. :-)
[21:18] <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:20] <jcsackett> EdwinGrubbs: sounds good. thanks, man.