[04:17] anyone? https://code.edge.launchpad.net/~thumper/launchpad/fix-upgrade-branch-failure/+merge/31114 [04:21] lifeless: are you technically a reviewer? [04:28] I've never left; I was one of the very first reviewers. [04:29] and I spoke with bac about whether he wanted any action to stop being emeritus, he didn't [04:29] other than poppig back in the roster etc - which I volunteered for anyhow [04:30] that said, I'm fucked [04:30] is it trivial ? [04:34] it looks ok except on line 45 you have 2 lines of VWS, in a class only one line between methods please [04:35] * wgrant loves the Vim plugin which points that out. [04:39] lifeless: no it isn't trivial [04:40] lifeless: even if we had trivial, I don't think this should be any more [04:40] lifeless: also, just to rerun failing tests, use testr run --failing ? [04:41] I don't mean [trivial], I meant 'small' [04:42] I agree that your patch wouldn't have been [trivial]! [04:42] just small-enough-for-fucked-brain to review [04:42] yes, testr run --failing [04:43] lifeless: I'm trying to make as many branches as I can to be trivial to review [04:43] No .testr.conf config file [04:43] ? [04:44] lifeless: I did a testr init; zcat ... | testr load [04:44] thumper: it looks in . not in ., .., ../.. etc [04:45] lifeless: I'm in . [04:45] hmm [04:45] is there a .testr.conf ? [04:45] no [04:45] what creates it? [04:45] its in the lp tree [04:45] has been for a while [04:46] its versioned [04:46] oh [04:47] so my rm was prolly bad then [04:47] yes [04:47] .testrepository - safe to delete [04:47] .testr.conf - do nae touch [04:47] bzr revert to the rescue [05:30] mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/stop-mirroring-hosted/+merge/31116 [05:30] mwhudson: this is the last of the request mirror branches [05:30] * mwhudson looks [05:30] mwhudson: the one that actually raises [05:32] yay [05:34] thumper: approved [05:34] with a minor note [05:35] mwhudson: ta === 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 [14:03] EdwinGrubbs: I was looking at your +needs-packaging branch. [14:03] EdwinGrubbs: The MP mentions a URL on staging that should timeout - is your branch already on staging? [14:03] or this for after it has landed? [14:04] jelmer: no, it's not on staging, but you are unlikely to be able to trigger a timeout on launchpad.dev. [14:04] Ah, of course - makes sense [14:46] jelmer, ping, have time for a simple bit of test suite refactoring? [14:46] as a review [14:46] :) [14:46] mars: Sure! [14:46] refactoring ftw [14:46] ok, It's on +activereviews === mars changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [mars] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:47] thanks jelmer === jelmer changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: mars || queue: [mars] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === jelmer changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: mars || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === gary_poster_ is now known as gary_poster [15:03] mars: Is it ok that the Mocker object is static and doesn't get cleaned up between tests? [15:06] jelmer, yes, I think it is a singleton Facade to mocker itself [15:07] That is what mocker.reset() is for I believe [15:07] had to experiment a bit to find that out [15:08] jelmer, you know, it could probably become a test case member as well [15:09] the docs don't really have a best practice there, short of using the MockerTestCase base class [15:12] Ah, ok - that makes sense [15:12] I guess the overhead of creating a Mocker when importing that module also isn't too bad [15:13] Is this the first place where we're using mocker? [15:14] it is [15:15] ideal case for it too [15:15] WindmillClient uses dynamic methods, so there is no base class you can inherit and override [15:16] you have to manually stub out every single method, or use mocks [15:24] mars: r=me, see some minor comments in the MP [15:26] jelmer, great, thanks for the review [15:26] I'll have another review, a one-liner, coming along in a few minutes [15:35] jelmer, time for one more? 2 lines edited: https://code.edge.launchpad.net/~mars/launchpad/activate-xvfb-error-log/+merge/31154 [15:35] ugh, diff updates are slow today === matsubara is now known as matsubara-lunch [15:54] * jelmer kicks xchat for not highlighting [15:54] mars: r=me [15:55] jelmer, thanks! [16:07] EdwinGrubbs: are you reviewing? [16:07] bac: yes === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jelmer, Edwin || reviewing: mars || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:08] EdwinGrubbs: would you mind looking at my private membership destroyer branch? sadly it is huge...almost 1400 lines [16:08] https://code.edge.launchpad.net/~bac/launchpad/bug-602773/+merge/31162 [16:08] bac: sure [16:27] bac: it doesn't look like validate_person does anything besides check IPerson.providedBY(). Is there something important it should be doing, or could it just be removed? [16:50] bac: did you see my comment above? === jelmer changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: mars || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [17:07] EdwinGrubbs, could you please review https://code.launchpad.net/~abentley/launchpad/reject-private-recipe/+merge/31171 ? [17:13] abentley: I can do that after lunch. [17:13] EdwinGrubbs, thanks === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: - || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === abentley changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: mars || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === EdwinGrubbs is now known as Edwin-lunch === abentley changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: mars || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mars changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [18:21] hi Edwin-lunch, sorry i didn't see your comment before i went to lunch. [18:23] Edwin-lunch, lifeless reviewed the one I asked you to, so please review https://code.edge.launchpad.net/~abentley/launchpad/new-score/+merge/31178 instead. === abentley changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: - || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [19:08] Edwin-lunch: back? === Edwin-lunch is now known as EdwinGrubbs [19:14] bac: I'm here [19:18] hi EdwinGrubbs...otp for a sec [19:20] EdwinGrubbs: yeah, i was unsure about keeping that person validator. you think it is useless? [19:21] i'm unclear how a non-person could get that far [19:22] bac: I doubt that a nonperson could get that far. [19:23] yeah, i think that test was just in there to protect the remaining checks in the old validator. probably overly defensive [19:24] thanks for the review. i'll remove that validator and land via ec2, as always [19:26] bac: ok, I'll mark it approved then. [19:26] thanks, EdwinGrubbs [19:56] Hi EdwinGrubbs, would you have time for https://code.edge.launchpad.net/~james-w/launchpad/copy-archive-job/+merge/28439 ? [19:57] james_w: I can work on that a little bit later today. [19:57] great, thanks === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: abentley || queue: [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:16] abentley: what is the queue_record.lastscore exactly? [20:17] EdwinGrubbs, it is the score that has been assigned to the build to determine its priority. "last" is hysterical raisins. [20:31] EdwinGrubbs: i've got a 37 line branch. may i add it to your queue? [20:31] bac: sure === bac changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: abentley || queue: [james_w, bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:31] thanks. https://code.edge.launchpad.net/~bac/launchpad/bug-607733/+merge/31195 [20:35] abentley: r=me [20:38] EdwinGrubbs, thanks. [20:54] EdwinGrubbs: you said earlier you would update https://code.edge.launchpad.net/~bac/launchpad/bug-602773/+merge/31162 -- could you do that now? [20:56] bac: oops, done. === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: james_w || queue: [bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-afk [23:33] james_w: review sent === 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 [23:34] thanks