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