[01:27] <lifeless> EdwinGrubbs: https://code.edge.launchpad.net/~lifeless/launchpad/registry/+merge/31805
[01:27] <lifeless> (if you're still oncall)
[02:10] <mwhudson> lifeless: looking
[02:14] <mwhudson> lifeless: you've seen TestCase.assertStatementCount ?
[02:15] <mwhudson> it's sort of a bit like but not quite like QueryCounter
[02:19] <lifeless> I looked for it
[02:19] <lifeless> but found QueryCounter
[02:19] <lifeless> oh yay two totally different ways of doing the same thing.
[02:20] <lifeless> uhm
[02:20] <lifeless> sigh, thanks.
[02:20] <lifeless> I think my refactoring is not harmful
[02:20] <lifeless> in fact, I think QueryCounter is useful because its aimed directly at the request, not at the storm layer
[02:20] <lifeless> so its useful for 'is this page request going to be ok'
[02:21] <lifeless> which, record_statements on allmembers is not a good answers for
[02:21] <mwhudson> ah, i just commented on the mp
[02:23] <mwhudson> lifeless: looking at the implementation, the two things are actually doing very similar things
[02:23] <mwhudson> also this makes me wince:
[02:23] <mwhudson> def clear_request_started():
[02:23] <mwhudson>     """Clear the request timer.  This function should be called when
[02:23] <mwhudson>     the request completes.
[02:23] <lifeless> where is that ?
[02:24] <mwhudson> canonical.launchpad.webapp.adapter
[02:24] <lifeless> oh heh
[02:24] <lifeless> nasty file that
[02:25] <mwhudson> yes
[02:26] <mwhudson> lifeless: anyway, perfectly happy to approve branch as is, just wanted you to know about the other way
[02:27] <mwhudson> lifeless: approved
[02:29] <lifeless> mwhudson: thanks!
[02:29] <lifeless> mwhudson: I think its ok to have two semanticly different things
[02:29] <lifeless> mwhudson: because I can imagine as we get more event driveny that they will be rather different
[02:31] <mwhudson> lifeless: i guess i'd be happier if get_request_statements did something closer to what its name suggested, it's not reaaaaaaally tied to the request at the moment
[02:32] <mwhudson> it's just 'queries issued in this thread between beginPublication and endPublication' or whatever
[02:32] <mwhudson> no getting around that with an implicit database connection i guess
[02:32] <lifeless> right
[02:32] <lifeless> so I can see that getting fixed
[02:32] <lifeless> e.g. if we get more twisted into the appservers (long poll!)
[02:32] <lifeless> or whatever
[05:34] <wgrant> jtv: Hi.
[05:34] <jtv> hi wgrant!
[05:34] <StevenK> wgrant: Stalker!
[05:34] <wgrant> jtv: Thanks for the review yesterday.
[05:35] <wgrant> jtv: But you approved but didn't approve it.
[05:35] <jtv> wgrant: ah yes, sorry… that's something we often do for ourselves.
[05:35] <wgrant> jtv: Yeah, I can't do that :)
[05:36] <jtv> I sort of got in the habit of letting the author set the status for themselves after they process any review suggestions I give them, but since I didn't do that either...
[05:36] <wgrant> Heh.
[05:36] <jtv> StevenK: if you say william's stalking me, note I'd been online for several hours.  Wouldn't be very efficient stalking.
[05:37] <jtv> wgrant: there, done.  Now I suppose you'll want me to land it as well?
[05:37] <wgrant> jtv: I want to run it past bigjools again this evening.
[05:37] <wgrant> So, please don't.
[05:38] <wgrant> Just in case, you know.
[05:38] <jtv> wgrant: so… just the sort of thing you want to defer the Approved status for?
[05:38] <wgrant> jtv: hmmm.
[05:38] <lifeless> so, approved should mean
[05:38] <lifeless> 'the reviewer does not need to see it again'
[05:38] <jtv> say mwhudson, would you be free for an oversized review?  Couldn't get anyone yesterday and it's starting to hold me up.
[05:39] <lifeless> which is != 'it is ready to land'
[05:39] <jtv> lifeless: an approved _vote_ means the reviewer doesn't have to see it again.  But multiple people are involved in an MP.
[05:39] <lifeless> jtv: that too is true
[05:39] <jtv> So the Approved status comes about when everyone who's planning to say anything about the branch, including the author, feels it's ready to ladn.
[05:39] <jtv> land.
[05:40] <lifeless> jtv: that does not match the model the code team have.
[05:40] <jtv> what do they do?
[05:40] <lifeless> the approved status is /intended/ to mean 'the branch is approved from the point of view of the approval policy for the project, whatever that may be'
[05:41] <jtv> lifeless: I should amend what I said to: anyone who's _supposed_ to say anything about the branch.
[05:41] <lifeless> the 'queued' status is used for 'land it now' - but is disabled in the web UI for a few reasons.
[05:43] <jtv> lifeless: I didn't even know that state existed.  What's wrong with defining approval policy the way I said?
[05:44] <lifeless> jtv: it implies a blocking transition where one may not be needed
[05:44] <jtv> May not.  If that needs to be established, you've got a blocking transition.
[05:45] <lifeless> so, on a per project, per review basis - sure
[05:45] <lifeless> you can say 'this proposal is not ready to approve' and approve it when it is ready.
[05:45] <mwhudson> jtv: i should probably do some linaro stuff really, is it large and complicated or just large?
[05:46] <jtv> mwhudson: ah yes, I forgot about linaro.  It's reasonably complicated, so I'll find some other suc^H^H^Hhelpful soul
[05:46] <lifeless> I was told when I did a bunch of reviews that the LP policy is 'approved status + needs-fixing vote' to say 'fix this up and then land it', and so on.
[05:48] <jtv> lifeless: I wasn't aware of that—I thought "needs fixing" meant "fix it and then come back to me."  Seems to match the privileges better: only the reviewer can change that vote but the engineer can usually change the status.
[05:51] <jtv> thumper, are you someone I can bother for an oversized review at the moment?
[05:51] <thumper> jtv: no
[05:51]  * jtv puts up the it's-alright-I-understand act
[05:52] <thumper> lifeless: if I say fix it and land, I vote approve with 'conditional' or something
[05:52] <jtv> Same here.
[05:52] <thumper> lifeless: to me needs-fixing means get back to the reviewer
[05:54] <jtv> lifeless: you know I hate to lean on you so heavily but is this oversized review something you could do?
[05:55] <jtv> It's this one: https://code.edge.launchpad.net/~jtv/launchpad/recife-traits/+merge/31729
[05:55] <lifeless> jtv: I'm playing catchup myself today :(
[05:55] <lifeless> jtv: is it mechanical
[05:55] <jtv> Alas, no
[05:55] <lifeless> jtv: or does it require thought?
[05:56] <jtv> Alas, yes
[05:56] <jtv> so that leaves… jml?
[05:56] <lifeless> jml, stub, danilo, all of eastern europe
[05:57] <jtv> I don't see any of those right now
[05:57] <lifeless> right, I'm talking candidates-while-you'l;-be-awake :)
[09:51] <lifeless> jelmer: jtv has a big patch he's begging for a review of
[09:53] <jelmer> k
[09:53] <lifeless> jtv: ^
[10:10] <jtv> lifeless: thanks otp
[10:18] <wgrant> jelmer: Could you review https://code.edge.launchpad.net/~wgrant/launchpad/destroy-publishedpackage/+merge/31813, when you have a moment?
[10:18] <jelmer> wgrant: yes, of course
[10:18] <lifeless> wgrant: I reviewed it from a code angle fwiw
[10:20] <jelmer> lifeless,wgrant: what other angle does it need review from?
[10:21] <lifeless> none AFAIK
[10:21] <wgrant> lifeless, jelmer: Ah, sorry, I'd assumed that was a DB review.
[10:21] <wgrant> lifeless: Can you land that, then, please?
[10:21] <lifeless> wgrant: e-rather-late; jelmer can :)
[10:22] <jelmer> sure, no problemo
[10:22] <wgrant> Thanks.
[10:23] <jtv> jelmer: thanks for putting me on the queue.  Overweight branch, with apologies.
[11:28] <jelmer> StevenK: You mention in your MP that IDistroSeries.initialiseFromParent() has nothing to do with DistroSeries in general, which puzzles me a bit.
[11:29] <jelmer> it seems like a reasonable thing to have on IDistroSeries, if only as a convenience method
[11:32] <StevenK> jelmer: It looked to bigjools and me as completly seperate ...
[11:33] <lifeless> FWIW
[11:33] <lifeless> it looks to me like its conceptually a class method
[11:33] <lifeless> as in
[11:33] <lifeless> it creates a new type X from another type X
[11:33] <lifeless> thats kindof like __new__
[11:33] <lifeless> if you squint
[11:34] <jelmer> lifeless: Yep, though it's called on a freshly created DistroSeries, it doesn't create one itself.
[11:34] <bigjools> i-f-p is a soyuz function that acts on a registry object
[11:35] <bigjools> it's *nothing* to do with Registry and should not live on IDistroSeries
[11:36] <lifeless> so IDistroSeries has nothing to do with registry ?
[11:36] <jelmer> lifeless: IDistroSeries is owned by registry, i-f-p is sozuz
[11:37] <lifeless> jelmer: I *still* don't get why
[11:37] <lifeless> but its late, and I'm this || far away from trolling by mistake
[11:37] <lifeless> so I'm going to stop work :)
[11:37] <bigjools> lifeless: you're twisting what I said
[11:38] <jelmer> bigjools, StevenK: Thanks. I still think it makes sense conceptually to have i-f-p or a convenience method that calls out to i-f-p on IDistroSeries, but I can understand why it's not there.
[11:38] <bigjools> i-f-p initialises a distroseries in a soyuz context
[11:38] <bigjools> our model objects are too big already
[11:38] <lifeless> bigjools: see above, really, epically tired and there is stuff I need to learn here but I won't after being up for 17 hours
[11:39] <bigjools> lifeless: !
[11:39] <bigjools> go to bed
[11:39] <lifeless> I *don't* want to troll by accident
[11:39] <lifeless> When I troll, I want to mean it :)
[11:39] <bigjools> go to your bed under that bridge over there
[11:55] <jelmer> StevenK: r=me
[11:56] <henninge> jelmer: I took jtv's branch
[11:57] <jelmer> henninge: ah, great - thanks
[12:33]  * jelmer lunches
[13:27] <jtv> henninge: any luck with that review?
[13:27] <jtv> (I'm pretty much EOD at this point)
[13:27] <jtv> So if you're stuck on anything, do say so soon!
[14:43] <henninge> jtv: sorry, still working on it.
[14:43] <jtv> No worries.  I'll eod then.
[14:46] <henninge> jtv: what's the purpose of MessageSideHelper?
[14:47] <henninge> jtv: couldn't those methods be simply on Translationmessage?
[14:47] <jtv> henninge: it's a bit legacy: it's where I extracted the Traits from.  One thing it does though is provide a place where the incumbent messages can be cached.
[14:47] <jtv> (Can't do that in the TM model class of course)
[14:47] <henninge> jtv: sorry, POTMsgSet, I guess.
[14:48] <jtv> Can't do it in any model class.
[14:48] <jtv> I'd like to avoid relying automatically on a method that queries for incumbent messages.
[14:49] <jtv> At the moment we do check that in makeCurrentUpstream and makeCurrentUbuntu; I'd prefer to have that check a bit higher up somewhere, or at least let the traits go to a lower-level setter.
[14:50] <jtv> That also means we won't need the duplication between those two methods—it'd be a single traits-based implementation.
[15:08] <henninge> jtv: we will have to continue that tomorrow morning. I still have to switch locations for the rest of the day.
[15:09] <jtv> henninge: and me for the night.  :)  Bis dann!
[15:09] <henninge> Tschüss
[16:34] <james_w> jelmer: https://code.edge.launchpad.net/~james-w/launchpad/stop-publishing-nagging/+merge/31851 please
[16:35] <jelmer> james_w, yessir
[16:37] <james_w> jelmer: oh, second revision pushed to remove the import too
[16:39] <jelmer> k
[17:17] <EdwinGrubbs> jelmer: can you review this branch for me? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-602385-register-project-from-sourcepackage-page/+merge/31856
[17:18] <jelmer> EdwinGrubbs: yes, sure.
[17:18] <jelmer> EdwinGrubbs: great to see a fix for that bug.
[17:19] <jelmer> EdwinGrubbs: There seems to be a (unnecessary?) conflict in that branch
[17:19] <jelmer> EdwinGrubbs: lib/lp/testing/factory.py
[17:39] <EdwinGrubbs> jelmer: I fixed the conflict and pushed it up.
[17:40] <jelmer> EdwinGrubbs: Thanks
[17:42] <jelmer> EdwinGrubbs, lib/lp/registry/model/sourcepackage.py has a print statement in it, is that intentional?
[17:51] <EdwinGrubbs> jelmer: no, that was a mistake, the changes I just pushed should have removed that.
[18:21] <jelmer> EdwinGrubbs: I'm off soon, once but I'll finish the review of your branch.
[18:21] <EdwinGrubbs> ok
[18:31] <jelmer> EdwinGrubbs: I've just followed up
[18:32] <EdwinGrubbs> thanks
[19:57] <abentley> rockstar, I can has rubberstamp? https://code.edge.launchpad.net/~bzr-loom-devs/bzr-loom/trunk/+merge/31873
[19:57] <rockstar> abentley, rs=me
[20:14] <abentley> rockstar, I can has rubberstamp? https://code.edge.launchpad.net/~bzr-hg/bzr-hg/trunk/+merge/31876
[20:14] <rockstar> abentley, rs=me
[20:29] <abentley> rockstar, how long does a branch upgrade usually take?
[20:30] <rockstar> abentley, depends on the size of the branch.
[20:30] <rockstar> abentley, in most cases, a few minutes maximum.  In other cases, hours.
[20:30] <abentley> rockstar, this branch is taking longer than I would expect: https://code.edge.launchpad.net/~launchpad-pqm/bzr-loom/trunk
[20:31] <abentley> rockstar, takes ~2 seconds locally.
[20:36] <rockstar> abentley, it might have other branch upgrade jobs in front of it.
[20:37] <rockstar> abentley, all it's really doing is copying the tree over a local file transport, and upgrading the branch.  I can't imagine it would be that slow.
[20:38] <abentley> rockstar, it's done now, but it did take 10 minutes or so.
[20:39] <rockstar> abentley, yeah, we need some better logging.  There's an open bug about that.
[20:40] <rockstar> abentley, maybe it's small enough that I can get to it sometime today.
[22:02] <james_w> if anyone is up for a review https://code.edge.launchpad.net/~james-w/launchpad/soyuz-factory-improvements/+merge/31890
[22:15] <james_w> https://code.edge.launchpad.net/~james-w/launchpad/no-more-sampledata-2/+merge/31891 is a follow-on from that one
[23:04] <rockstar> thumper, i can has revyooz?  https://code.edge.launchpad.net/~rockstar/launchpad/recipe-build-table/+merge/31882
[23:05]  * mwhudson also has a review request, for a bug that's breaking staging and will break edge: https://code.edge.launchpad.net/~mwhudson/launchpad/disable-vostok-when-not-configured/+merge/31901
[23:06] <rockstar> mwhudson, we just talked about this on our standup.
[23:06] <rockstar> mwhudson, r=me
[23:08] <mwhudson> rockstar: thanks