| lifeless | EdwinGrubbs: https://code.edge.launchpad.net/~lifeless/launchpad/registry/+merge/31805 | 01:27 |
|---|---|---|
| lifeless | (if you're still oncall) | 01:27 |
| mwhudson | lifeless: looking | 02:10 |
| mwhudson | lifeless: you've seen TestCase.assertStatementCount ? | 02:14 |
| mwhudson | it's sort of a bit like but not quite like QueryCounter | 02:15 |
| 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:19 |
| 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:20 |
| lifeless | which, record_statements on allmembers is not a good answers for | 02:21 |
| mwhudson | ah, i just commented on the mp | 02:21 |
| 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:23 |
| mwhudson | canonical.launchpad.webapp.adapter | 02:24 |
| lifeless | oh heh | 02:24 |
| lifeless | nasty file that | 02:24 |
| mwhudson | yes | 02:25 |
| mwhudson | lifeless: anyway, perfectly happy to approve branch as is, just wanted you to know about the other way | 02:26 |
| mwhudson | lifeless: approved | 02:27 |
| 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:29 |
| 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:31 |
| 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 | 02:32 |
| === 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 | ||
| wgrant | jtv: Hi. | 05:34 |
| jtv | hi wgrant! | 05:34 |
| StevenK | wgrant: Stalker! | 05:34 |
| wgrant | jtv: Thanks for the review yesterday. | 05:34 |
| 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:35 |
| 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:36 |
| 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:37 |
| 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:38 |
| 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:39 |
| 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:40 |
| 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:41 |
| jtv | lifeless: I didn't even know that state existed. What's wrong with defining approval policy the way I said? | 05:43 |
| 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:44 |
| 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:45 |
| 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:46 |
| 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:48 |
| 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:51 | |
| 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:52 |
| jtv | lifeless: you know I hate to lean on you so heavily but is this oversized review something you could do? | 05:54 |
| 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:55 |
| jtv | Alas, yes | 05:56 |
| jtv | so that leaves… jml? | 05:56 |
| lifeless | jml, stub, danilo, all of eastern europe | 05:56 |
| jtv | I don't see any of those right now | 05:57 |
| lifeless | right, I'm talking candidates-while-you'l;-be-awake :) | 05:57 |
| === 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 | ||
| lifeless | jelmer: jtv has a big patch he's begging for a review of | 09:51 |
| jelmer | k | 09:53 |
| lifeless | jtv: ^ | 09:53 |
| jtv | lifeless: thanks otp | 10:10 |
| 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:18 |
| === 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 | ||
| jelmer | lifeless,wgrant: what other angle does it need review from? | 10:20 |
| lifeless | none AFAIK | 10:21 |
| wgrant | lifeless, jelmer: Ah, sorry, I'd assumed that was a DB review. | 10:21 |
| === 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 | ||
| wgrant | lifeless: Can you land that, then, please? | 10:21 |
| lifeless | wgrant: e-rather-late; jelmer can :) | 10:21 |
| jelmer | sure, no problemo | 10:22 |
| wgrant | Thanks. | 10:22 |
| jtv | jelmer: thanks for putting me on the queue. Overweight branch, with apologies. | 10:23 |
| === adeuring1 is now known as adeuring | ||
| jelmer | StevenK: You mention in your MP that IDistroSeries.initialiseFromParent() has nothing to do with DistroSeries in general, which puzzles me a bit. | 11:28 |
| jelmer | it seems like a reasonable thing to have on IDistroSeries, if only as a convenience method | 11:29 |
| StevenK | jelmer: It looked to bigjools and me as completly seperate ... | 11:32 |
| 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:33 |
| 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:34 |
| bigjools | it's *nothing* to do with Registry and should not live on IDistroSeries | 11:35 |
| lifeless | so IDistroSeries has nothing to do with registry ? | 11:36 |
| jelmer | lifeless: IDistroSeries is owned by registry, i-f-p is sozuz | 11:36 |
| 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:37 |
| 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:38 |
| 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:39 |
| jelmer | StevenK: r=me | 11:55 |
| === 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 | ||
| henninge | jelmer: I took jtv's branch | 11:56 |
| jelmer | henninge: ah, great - thanks | 11:57 |
| === 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 | ||
| * jelmer lunches | 12:33 | |
| === Ursinha-afk is now known as Ursinha | ||
| === matsubara-afk is now known as matsubara | ||
| === mrevell-lunch is now known as mrevell | ||
| 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! | 13:27 |
| henninge | jtv: sorry, still working on it. | 14:43 |
| jtv | No worries. I'll eod then. | 14:43 |
| henninge | jtv: what's the purpose of MessageSideHelper? | 14:46 |
| 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:47 |
| 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:48 |
| 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:49 |
| jtv | That also means we won't need the duplication between those two methods—it'd be a single traits-based implementation. | 14:50 |
| henninge | jtv: we will have to continue that tomorrow morning. I still have to switch locations for the rest of the day. | 15:08 |
| jtv | henninge: and me for the night. :) Bis dann! | 15:09 |
| henninge | Tschüss | 15:09 |
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-brb | ||
| james_w | jelmer: https://code.edge.launchpad.net/~james-w/launchpad/stop-publishing-nagging/+merge/31851 please | 16:34 |
| jelmer | james_w, yessir | 16:35 |
| === 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 | ||
| james_w | jelmer: oh, second revision pushed to remove the import too | 16:37 |
| jelmer | k | 16:39 |
| === salgado is now known as salgado-lunch | ||
| === Ursinha-brb is now known as Ursinha | ||
| 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:17 |
| jelmer | EdwinGrubbs: yes, sure. | 17:18 |
| jelmer | EdwinGrubbs: great to see a fix for that bug. | 17:18 |
| jelmer | EdwinGrubbs: There seems to be a (unnecessary?) conflict in that branch | 17:19 |
| jelmer | EdwinGrubbs: lib/lp/testing/factory.py | 17:19 |
| === matsubara is now known as matsubara-lunch | ||
| EdwinGrubbs | jelmer: I fixed the conflict and pushed it up. | 17:39 |
| jelmer | EdwinGrubbs: Thanks | 17:40 |
| jelmer | EdwinGrubbs, lib/lp/registry/model/sourcepackage.py has a print statement in it, is that intentional? | 17:42 |
| === salgado-lunch is now known as salgado | ||
| EdwinGrubbs | jelmer: no, that was a mistake, the changes I just pushed should have removed that. | 17:51 |
| === 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 | ||
| jelmer | EdwinGrubbs: I'm off soon, once but I'll finish the review of your branch. | 18:21 |
| EdwinGrubbs | ok | 18:21 |
| === matsubara-lunch is now known as matsubara | ||
| jelmer | EdwinGrubbs: I've just followed up | 18:31 |
| EdwinGrubbs | thanks | 18:32 |
| 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 | 19:57 |
| 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:14 |
| abentley | rockstar, how long does a branch upgrade usually take? | 20:29 |
| 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:30 |
| abentley | rockstar, takes ~2 seconds locally. | 20:31 |
| rockstar | abentley, it might have other branch upgrade jobs in front of it. | 20:36 |
| 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:37 |
| abentley | rockstar, it's done now, but it did take 10 minutes or so. | 20:38 |
| rockstar | abentley, yeah, we need some better logging. There's an open bug about that. | 20:39 |
| rockstar | abentley, maybe it's small enough that I can get to it sometime today. | 20:40 |
| james_w | if anyone is up for a review https://code.edge.launchpad.net/~james-w/launchpad/soyuz-factory-improvements/+merge/31890 | 22:02 |
| james_w | https://code.edge.launchpad.net/~james-w/launchpad/no-more-sampledata-2/+merge/31891 is a follow-on from that one | 22:15 |
| === salgado is now known as salgado-afk | ||
| === matsubara is now known as matsubara-afk | ||
| rockstar | thumper, i can has revyooz? https://code.edge.launchpad.net/~rockstar/launchpad/recipe-build-table/+merge/31882 | 23:04 |
| * 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:05 | |
| rockstar | mwhudson, we just talked about this on our standup. | 23:06 |
| rockstar | mwhudson, r=me | 23:06 |
| mwhudson | rockstar: thanks | 23:08 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!