/srv/irclogs.ubuntu.com/2010/07/28/#launchpad-reviews.txt

thumperanyone? https://code.edge.launchpad.net/~thumper/launchpad/fix-upgrade-branch-failure/+merge/3111404:17
thumperlifeless: are you technically a reviewer?04:21
lifelessI've never left; I was one of the very first reviewers.04:28
lifelessand I spoke with bac about whether he wanted any action to stop being emeritus, he didn't04:29
lifelessother than poppig back in the roster etc - which I volunteered for anyhow04:29
lifelessthat said, I'm fucked04:30
lifelessis it trivial ?04:30
lifelessit looks ok except on line 45 you have 2 lines of VWS, in a class only one line between methods please04:34
* wgrant loves the Vim plugin which points that out.04:35
thumperlifeless: no it isn't trivial04:39
thumperlifeless: even if we had trivial, I don't think this should be any more04:40
thumperlifeless: also, just to rerun failing tests, use testr run --failing ?04:40
lifelessI don't mean [trivial], I meant 'small'04:41
lifelessI agree that your patch wouldn't have been [trivial]!04:42
lifelessjust small-enough-for-fucked-brain to review04:42
lifelessyes, testr run --failing04:42
thumperlifeless: I'm trying to make as many branches as I can to be trivial to review04:43
thumperNo .testr.conf config file04:43
thumper?04:43
thumperlifeless: I did a testr init; zcat ... | testr load04:44
lifelessthumper: it looks in . not in ., .., ../.. etc04:44
thumperlifeless: I'm in .04:45
lifelesshmm04:45
lifelessis there a .testr.conf ?04:45
thumperno04:45
thumperwhat creates it?04:45
lifelessits in the lp tree04:45
lifelesshas been for a while04:45
lifelessits versioned04:46
thumperoh04:46
thumperso my rm was prolly bad then04:47
lifelessyes04:47
lifeless.testrepository - safe to delete04:47
lifeless.testr.conf - do nae touch04:47
thumperbzr revert to the rescue04:47
thumpermwhudson: https://code.edge.launchpad.net/~thumper/launchpad/stop-mirroring-hosted/+merge/3111605:30
thumpermwhudson: this is the last of the request mirror branches05:30
* mwhudson looks05:30
thumpermwhudson: the one that actually raises05:30
mwhudsonyay05:32
mwhudsonthumper: approved05:34
mwhudsonwith a minor note05:34
thumpermwhudson: ta05: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
jelmerEdwinGrubbs: I was looking at your +needs-packaging branch.14:03
jelmerEdwinGrubbs: The MP mentions a URL on staging that should timeout - is your branch already on staging?14:03
jelmeror this for after it has landed?14:03
EdwinGrubbsjelmer: no, it's not on staging, but you are unlikely to be able to trigger a timeout on launchpad.dev.14:04
jelmerAh, of course - makes sense14:04
marsjelmer, ping, have time for a simple bit of test suite refactoring?14:46
marsas a review14:46
mars:)14:46
jelmermars: Sure!14:46
jelmerrefactoring ftw14:46
marsok, It's on +activereviews14: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
marsthanks jelmer14: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
jelmermars: Is it ok that the Mocker object is static and doesn't get cleaned up between tests?15:03
marsjelmer, yes, I think it is a singleton Facade to mocker itself15:06
marsThat is what mocker.reset() is for I believe15:07
marshad to experiment a bit to find that out15:07
marsjelmer, you know, it could probably become a test case member as well15:08
marsthe docs don't really have a best practice there, short of using the MockerTestCase base class15:09
jelmerAh, ok - that makes sense15:12
jelmerI guess the overhead of creating a Mocker when importing that module also isn't too bad15:12
jelmerIs this the first place where we're using mocker?15:13
marsit is15:14
marsideal case for it too15:15
marsWindmillClient uses dynamic methods, so there is no base class you can inherit and override15:15
marsyou have to manually stub out every single method, or use mocks15:16
jelmermars: r=me, see some minor comments in the MP15:24
marsjelmer, great, thanks for the review15:26
marsI'll have another review, a one-liner, coming along in a few minutes15:26
marsjelmer, time for one more?  2 lines edited:  https://code.edge.launchpad.net/~mars/launchpad/activate-xvfb-error-log/+merge/3115415:35
marsugh, diff updates are slow today15:35
=== matsubara is now known as matsubara-lunch
* jelmer kicks xchat for not highlighting15:54
jelmermars: r=me15:54
marsjelmer, thanks!15:55
bacEdwinGrubbs: are you reviewing?16:07
EdwinGrubbsbac: yes16: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
bacEdwinGrubbs: would you mind looking at my private membership destroyer branch?  sadly it is huge...almost 1400 lines16:08
bachttps://code.edge.launchpad.net/~bac/launchpad/bug-602773/+merge/3116216:08
EdwinGrubbsbac: sure16:08
EdwinGrubbsbac: 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
EdwinGrubbsbac: 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
abentleyEdwinGrubbs, could you please review https://code.launchpad.net/~abentley/launchpad/reject-private-recipe/+merge/31171 ?17:07
EdwinGrubbsabentley: I can do that after lunch.17:13
abentleyEdwinGrubbs, thanks17: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
bachi Edwin-lunch, sorry i didn't see your comment before i went to lunch.18:21
abentleyEdwin-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
bacEdwin-lunch: back?19:08
=== Edwin-lunch is now known as EdwinGrubbs
EdwinGrubbsbac: I'm here19:14
bachi EdwinGrubbs...otp for a sec19:18
bacEdwinGrubbs: yeah, i was unsure about keeping that person validator.  you think it is useless?19:20
baci'm unclear how a non-person could get that far19:21
EdwinGrubbsbac: I doubt that a nonperson could get that far.19:22
bacyeah, i think that test was just in there to protect the remaining checks in the old validator.  probably overly defensive19:23
bacthanks for the review.  i'll remove that validator and land via ec2, as always19:24
EdwinGrubbsbac: ok, I'll mark it approved then.19:26
bacthanks, EdwinGrubbs19:26
james_wHi EdwinGrubbs, would you have time for https://code.edge.launchpad.net/~james-w/launchpad/copy-archive-job/+merge/28439 ?19:56
EdwinGrubbsjames_w: I can work on that a little bit later today.19:57
james_wgreat, thanks19: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
EdwinGrubbsabentley: what is the queue_record.lastscore exactly?20:16
abentleyEdwinGrubbs, it is the score that has been assigned to the build to determine its priority.  "last" is hysterical raisins.20:17
bacEdwinGrubbs: i've got a 37 line branch.  may i add it to your queue?20:31
EdwinGrubbsbac: sure20: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
bacthanks.  https://code.edge.launchpad.net/~bac/launchpad/bug-607733/+merge/3119520:31
EdwinGrubbsabentley: r=me20:35
abentleyEdwinGrubbs, thanks.20:38
bacEdwinGrubbs: you said earlier you would update https://code.edge.launchpad.net/~bac/launchpad/bug-602773/+merge/31162 -- could you do that now?20:54
EdwinGrubbsbac: 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
EdwinGrubbsjames_w: review sent23: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_wthanks23:34

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!