/srv/irclogs.ubuntu.com/2010/04/09/#launchpad-reviews.txt

jelmerrockstar: Hi! If you're still reviewing, any chance you can have a look at this mp: https://code.launchpad.net/~jelmer/launchpad/upgrade-sourcecode/+merge/2303400:12
=== sinzui changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
sinzuirockstar, thumper: can either of you review https://code.launchpad.net/~sinzui/launchpad/series-branch-target/+merge/2305600:20
wgrantCan someone please land https://code.edge.launchpad.net/~wgrant/launchpad/move-rescueiflost-tests/+merge/22737?00:23
mwhudsonman it's all ask ask ask in here today00:29
mwhudsonjelmer: did you consider just deleting and re-pulling in the cross-format case?00:29
mwhudsonit's simpler and probably quicker00:29
mwhudsonwgrant: is this one of the ones that passed ec2 yesterday but got foiled by testfix?00:30
wgrantmwhudson: No, noodles landed all of them overnight.00:31
wgrantThis one needs EC2ing too.00:31
mwhudsonwgrant: OK00:31
jelmermwhudson: that's a good point; do you reckon that's a better idea than upgrading? I guess it's a bandwidth vs cpu tradeoff00:32
mwhudsonjelmer: my thinking is that they're all small branches so that simplicity is better00:34
jelmermwhudson: I wonder if removing the branch is simpler in this case though; it'll involve checking that the tree is unchanged, etc00:34
mwhudsonjelmer: hm, fair point00:35
mwhudsonjelmer: you seem to have lost the 'don't pull if the revision id is the same' thing00:38
mwhudsonbut i guess that's not much of an optimization really00:38
jelmermwhudson: Bazaar really should be taking care of that case itself00:39
jelmerat least imnsho :-)00:39
mwhudsonquite00:40
mwhudsonjelmer: approved00:42
jelmermwhudson: thanks!00:43
mwhudsonwgrant: your branch has conflicts with devel it seems00:55
wgrantmwhudson: Bah.00:55
* wgrant merges.00:55
* jpds has https://code.launchpad.net/~jpds/launchpad/stormify-distribution-bits/+merge/23060 up for grabs.01:12
=== jpds changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [sinzui,jpds] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
mwhudsonwgrant: pushed yet?  should i run ec2 land again?01:13
wgrantmwhudson: Just fighting buildout so I can rerun the involved tests.01:14
mwhudsonheh heh01:15
* wgrant stabs $SOMETHING in the face.01:17
wgrantlazr.config.interfaces.NoConfigError: No config with name: initZopeless config overlay.01:17
wgrantYES BUT WHY?01:17
jpds /1101:17
wgrantSo it turns out that the error actually meant that I needed to fix permissions on the account table.01:20
wgrantThat makes sense.01:20
wgrantmwhudson: Pushed.01:38
wgrantmwhudson: Did you kick that ec2 off yet?02:23
mwhudsonwgrant: yeah, sorry02:23
mwhudsonwgrant: it's headless now02:23
wgrantThanks.02:23
wgrantGreat.02:23
=== jtv is now known as jtv-afk
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: jpds || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
adeuringOn call: adeuring || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews09:00
mwhudsonadeuring: you missed a /topic there09:04
adeuringmwhudson: arghh... thanks!09:05
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
gmbadeuring, Morning. Can you give me a code review on https://code.edge.launchpad.net/~gmb/launchpad/add-next_check-to-bugwatch-pages-bug-558410/+merge/23080?10:41
adeuringgmb: sure10:41
gmbnoodles775, Any chance of a quick UI review of https://code.edge.launchpad.net/~gmb/launchpad/add-next_check-to-bugwatch-pages-bug-558410/+merge/23080? I can provide screenshots if needed; it's a trivial change.10:41
noodles775gmb: I can take a look, but if it's non-urgent, it'd be great to get one of the others (rockstar, for eg), to do an initial review and I'll do the second one?10:45
noodles775Although, if it's that trivial, might not be worth it.10:45
gmbnoodles775, Well, it's just a case of adding data to the BugWatch +edit and BugTracker overview pages (lastchecked and next_check dates for bug watches, basically)10:45
gmbSo, your call.10:46
noodles775OK, I'll just do it then.10:46
gmbThanks.10:46
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: gmb || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
adeuringgmb: r=me10:57
gmbnoodles775, FTR, I have no idea why I've called next_check "Scheduled" on BugWatch+edit but called it "Next check" on the BugTracker overview. I think we should probably stick to one label, agreed?10:57
gmbadeuring, Thanks10:57
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: gmb || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
deryckHi adeuring.  I have one for the queue, when you can look at it.  https://code.edge.launchpad.net/~deryck/launchpad/not-notified-someone-else-subscribed-494257/+merge/2304411:32
=== deryck changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [sinzui, deryck] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
adeuringderyck: surw, I'll look11:32
deryckadeuring, thanks!11:32
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: deryck || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
adeuringderyck: overall, you branch looks good. I just wondered if it makes sense to explicitly test that bug.subscribe(..., send_notifications=False) does indeed not send notifications.11:49
deryckadeuring, ah, good point.11:50
adeuringderyck: OK, so r=me11:50
deryckadeuring, thanks!  I'll add the test here in a moment.11:50
gmbnoodles775, Thanks for the review.11:56
noodles775np.11:57
=== stub1 is now known as stub
maxbHello OCR, I'd like to enqueue https://code.edge.launchpad.net/~maxb/launchpad/ignored-asserts/+merge/18889 please, which is up for re-review after a tweak to the tests.12:50
=== maxb changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: deryck || queue: [sinzui,maxb(ignored-asserts)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
adeuringmaxb: sure, I'll look.12:54
adeuringmaxb: Or do you want gmb to cióntinue the review=12:54
adeurings/=/?/12:54
maxbIn this case, I don't think there's enough context for it to be worth me seeking out gmb - I'd rather get it reviewed today :-)12:55
adeuringfair eonough12:56
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: maxb(ignored-asserts) || queue: [sinzui,] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
maxbAh, I see gmb was around earlier - I only looked at the fact that he is currently marked away. Never mind, there really isn't much prior context to this branch.12:58
=== matsubara-afk is now known as matsubara
adeuringmaxb: r=me. As Graham said, good catch!13:15
maxbadeuring: Thanks, could you land it for me?13:15
adeuringmaxb: sure13:15
maxbexcellent. one more baby step towards python 2.613:16
maxboh, I'm supposed to set a commit message in the MP, aren't I13:16
maxbdone13:17
=== sinzui changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: maxb(ignored-asserts) || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
gary_posteradeuring: could you look at https://code.edge.launchpad.net/~gary/launchpad/small/+merge/23052 ?  This was a quick fix for a problem abentley found on karmic14:11
adeuringgary_poster: sure14:11
gary_posterthank you14:11
abentleygary_poster, adeuring: fix verified on my system.14:11
gary_posterthanks abentley14:12
adeuringgary_poster: r=me14:15
gary_posterthank you adeuring!14:15
=== salgado is now known as salgado-off
jpdsadeuring: Hi, did you ec2 land my branch?15:46
adeuringjpds: sorry, forgot it... Will do that now.15:46
jmladeuring: could you please review https://code.edge.launchpad.net/~jml/launchpad/more-services/+merge/23107 – it's a branch with a few simple cleanups?15:52
adeuringjml: sure15:53
jmlthanks.15:53
jpdsadeuring: No problem, thanks!15:56
allenapadeuring: Could you rubber-stamp a proposal for me please? It's a cherry-pick of an already-reviewed change. https://code.edge.launchpad.net/~allenap/launchpad/dynamic-batch-size-bug-546085-devel/+merge/2310516:06
adeuringallenap: done16:09
allenapadeuring: Thanks :)16:10
allenapadeuring: Could you try changing the review type to "code", or add an approve for "code" (or nothing)? bzr lp-land complains at the moment. (And if that doesn't work I'll land it by hand.)16:14
adeuringallenap: done. (sorry for the delay -- was on skype)16:22
allenapadeuring: Thanks :)16:22
=== matsubara is now known as matsubara-lunch
adeuringjml: r=me16:32
jmladeuring, thanks.16:34
allenapadeuring: Do you have time for another? Should be straightforward I hope. https://code.edge.launchpad.net/~allenap/launchpad/dont-update-dupe-bugs-bug-511276/+merge/2311316:34
adeuringallenap: yes16:35
adeuringallenap: r=me16:46
allenapadeuring: Thanks!16:46
allenapadeuring: I've got a silly one for you now :) https://code.edge.launchpad.net/~allenap/launchpad/dynamic-batch-size-bug-546085-db-devel-preempt-conflict/+merge/2311416:50
adeuringallenap: rs=me16:51
allenapadeuring: Thanks :)16:51
=== matsubara-lunch is now known as matsubara
=== deryck is now known as deryck[lunch]
=== gary_poster is now known as gary_lunch
=== deryck[lunch] is now known as deryck
=== gary_lunch is now known as gary_poster
=== deryck changed the topic of #launchpad-reviews to: On call: || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
deryckabel is gone.  Is anyone else reviewing this afternoon?19:45
deryckrockstar or abentley -- could I ask a review from one of you fine gentleman?  It's to do with linking branches on a bug page.19:48
abentleyderyck, sure.19:52
deryckabentley, thanks!  https://code.edge.launchpad.net/~deryck/launchpad/no-linky-abandoned-branches-559325/+merge/2313519:52
abentleyderyck, was there a preimplementation call?19:55
deryckabentley, no.  I just did it on a whim at lunch.19:56
maxbderyck: I question the appropriateness of this change because it hides potential partial work done on a bug that someone else could rescue and complete19:56
abentleyderyck, I'm not sure this is the right solution.19:56
deryckabentley, ah, ok.  fair enough.  I did discuss the proposed solution with jml in the bug.  And he agreed with the approach.19:56
abentleyderyck, yes, I saw that.  Still, he's not on the code team anymore.19:58
abentleyderyck, my reasons are much like maxb's.19:58
deryckabentley, I didn't see maxb's comment?  Where is that?19:59
abentleyderyck, and I'm not sure that abandoning a branch is a clear indication that you want to never get any questions about it.19:59
abentleyderyck, in channel, 4 minutes ago.19:59
deryckah, sorry. thought that was you :-)20:00
deryckthe red bled together in xchat :-)20:00
abentleyperhaps appropriate in this case.20:01
deryckabentley, so your argument would be if you don't want the connection then unlink the branch?20:02
abentleyderyck, that seems suboptimal too.20:03
abentleyderyck, is it true that you never want to receive any questions about the branch, or do you just not want to be asked whether you're working on it?20:03
maxbWould it not be sufficient to display the branch's "Abandoned" status alongside its name in the bug "Related branches" list?20:04
deryckabentley, I would guess more the latter.  But I think it's hard to presume what anyone means by "abandoned" really.20:04
deryckmaxb, it's already listed.20:04
deryckmaxb, sorry, I'm wrong20:05
deryckwas thinking of rejected.20:05
maxbMP status is shown, branch status is nort20:05
maxb*not20:05
deryckright20:05
deryckyeah, maybe that's the right fix.20:05
abentleyderyck, so I think branch status should be shown, and we should maybe reduce the visibility of the merge proposal.20:06
abentleyderyck, but I'd be interested to hear what rockstar thinks.20:06
maxbI like the idea of reducing the visibility of the MP20:07
deryckabentley, I can go along with your suggestion.  but would be interested in rockstar's opinion, too.  I can go either way.  It makes sense to me to drop it completely, too.20:07
deryckbut I'm thinking as a bugs guy obviously :-)20:08
maxbDrop what? the MP or the entire branch?20:08
deryckentire branch per my fix now20:08
maxbJust because someone has abandoned a branch does not mean that it does not contain useful work20:09
abentleyderyck, if I'm not mistaken, the reason the branch status isn't shown is because the MP *is* shown.20:09
deryckmaxb, I don't disagree with you.  I'm saying I can see both sides.  Abandoned branches could also be useless, as well as they could be useful.20:10
maxbBetter to err on the side of not losing visibility on potential useful code, IMO20:11
deryckabentley, ok.  I don't think branch status is really useful unless it's abandoned.  So if we go this route, maybe we should treated abandoned branches differently, i.e. show branch status and a link to MP but no info on the MP?20:11
deryckmaxb, true.  but my assumption is someone marked it "abandoned" precisely because it wasn't useful.20:12
abentleyderyck, we use the "merged" status very heavily.  Others not so much.20:13
maxb"I have stopped work on this branch" doesn't mean the same as "There's no useful work in this branch"20:13
abentleyderyck, that's not what I would assume.  I would assume they didn't have time for it, or maybe the reviewer was being a jerk and obstructing it.20:14
deryckmaxb, but does ABANDONED always equal "I have stopped work on this branch?"  If so, how do you know that?  :-)  That's all I mean.  I'm not intending to argue with you, really. :-)20:14
abentleyderyck, yes, that's what it means.20:14
maxbWell, that's what the English word means to me :-)20:15
deryckfair enough.  I take your points.  But I don't have a desire to pursue it beyond this.  it was a lunch time fix for me.  I like removing some clutter from the bug page, but if we don't agree is right, I won't pursue it further myself.20:17
deryckThanks anyway, abentley, for the review.20:17
abentleyderyck, very well.  Sorry it didn't work out.20:18
deryckno worries20:18
* rockstar sees that he was apparently pinged in this channel while at lunch21:01
abentleyrockstar, don't worry about it.21:05
rockstarabentley, yeah, it looks like there was consensus.21:06
abentleyrockstar, well, more like resolution.21:06
rockstarabentley, I'd be all for hiding abandoned branches on a bug that have a branch that actually fixed the bug (and was landed)21:10
rockstarIn that case, I'd only show the one that fixed the issue.21:10
abentleyrockstar, chat?21:15
rockstarabentley, oh, it's getting to be that time huh?21:16
rockstarabentley, can you give me 5 minutes to get my tests passing?21:16
abentleyrockstar, sure.21:16
=== matsubara is now known as matsubara-afk

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