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