jelmer | rockstar: 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/23034 | 00: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 | ||
sinzui | rockstar, thumper: can either of you review https://code.launchpad.net/~sinzui/launchpad/series-branch-target/+merge/23056 | 00:20 |
wgrant | Can someone please land https://code.edge.launchpad.net/~wgrant/launchpad/move-rescueiflost-tests/+merge/22737? | 00:23 |
mwhudson | man it's all ask ask ask in here today | 00:29 |
mwhudson | jelmer: did you consider just deleting and re-pulling in the cross-format case? | 00:29 |
mwhudson | it's simpler and probably quicker | 00:29 |
mwhudson | wgrant: is this one of the ones that passed ec2 yesterday but got foiled by testfix? | 00:30 |
wgrant | mwhudson: No, noodles landed all of them overnight. | 00:31 |
wgrant | This one needs EC2ing too. | 00:31 |
mwhudson | wgrant: OK | 00:31 |
jelmer | mwhudson: that's a good point; do you reckon that's a better idea than upgrading? I guess it's a bandwidth vs cpu tradeoff | 00:32 |
mwhudson | jelmer: my thinking is that they're all small branches so that simplicity is better | 00:34 |
jelmer | mwhudson: I wonder if removing the branch is simpler in this case though; it'll involve checking that the tree is unchanged, etc | 00:34 |
mwhudson | jelmer: hm, fair point | 00:35 |
mwhudson | jelmer: you seem to have lost the 'don't pull if the revision id is the same' thing | 00:38 |
mwhudson | but i guess that's not much of an optimization really | 00:38 |
jelmer | mwhudson: Bazaar really should be taking care of that case itself | 00:39 |
jelmer | at least imnsho :-) | 00:39 |
mwhudson | quite | 00:40 |
mwhudson | jelmer: approved | 00:42 |
jelmer | mwhudson: thanks! | 00:43 |
mwhudson | wgrant: your branch has conflicts with devel it seems | 00:55 |
wgrant | mwhudson: 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 | ||
mwhudson | wgrant: pushed yet? should i run ec2 land again? | 01:13 |
wgrant | mwhudson: Just fighting buildout so I can rerun the involved tests. | 01:14 |
mwhudson | heh heh | 01:15 |
* wgrant stabs $SOMETHING in the face. | 01:17 | |
wgrant | lazr.config.interfaces.NoConfigError: No config with name: initZopeless config overlay. | 01:17 |
wgrant | YES BUT WHY? | 01:17 |
jpds | /11 | 01:17 |
wgrant | So it turns out that the error actually meant that I needed to fix permissions on the account table. | 01:20 |
wgrant | That makes sense. | 01:20 |
wgrant | mwhudson: Pushed. | 01:38 |
wgrant | mwhudson: Did you kick that ec2 off yet? | 02:23 |
mwhudson | wgrant: yeah, sorry | 02:23 |
mwhudson | wgrant: it's headless now | 02:23 |
wgrant | Thanks. | 02:23 |
wgrant | Great. | 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 | ||
adeuring | On call: adeuring || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | 09:00 |
mwhudson | adeuring: you missed a /topic there | 09:04 |
adeuring | mwhudson: 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 | ||
gmb | adeuring, 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 |
adeuring | gmb: sure | 10:41 |
gmb | noodles775, 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 |
noodles775 | gmb: 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 |
noodles775 | Although, if it's that trivial, might not be worth it. | 10:45 |
gmb | noodles775, 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 |
gmb | So, your call. | 10:46 |
noodles775 | OK, I'll just do it then. | 10:46 |
gmb | Thanks. | 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 | ||
adeuring | gmb: r=me | 10:57 |
gmb | noodles775, 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 |
gmb | adeuring, Thanks | 10: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 | ||
deryck | Hi 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/23044 | 11: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 | ||
adeuring | deryck: surw, I'll look | 11:32 |
deryck | adeuring, 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 | ||
adeuring | deryck: 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 |
deryck | adeuring, ah, good point. | 11:50 |
adeuring | deryck: OK, so r=me | 11:50 |
deryck | adeuring, thanks! I'll add the test here in a moment. | 11:50 |
gmb | noodles775, Thanks for the review. | 11:56 |
noodles775 | np. | 11:57 |
=== stub1 is now known as stub | ||
maxb | Hello 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 | ||
adeuring | maxb: sure, I'll look. | 12:54 |
adeuring | maxb: Or do you want gmb to cióntinue the review= | 12:54 |
adeuring | s/=/?/ | 12:54 |
maxb | In 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 |
adeuring | fair eonough | 12: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 | ||
maxb | Ah, 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 | ||
adeuring | maxb: r=me. As Graham said, good catch! | 13:15 |
maxb | adeuring: Thanks, could you land it for me? | 13:15 |
adeuring | maxb: sure | 13:15 |
maxb | excellent. one more baby step towards python 2.6 | 13:16 |
maxb | oh, I'm supposed to set a commit message in the MP, aren't I | 13:16 |
maxb | done | 13: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_poster | adeuring: 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 karmic | 14:11 |
adeuring | gary_poster: sure | 14:11 |
gary_poster | thank you | 14:11 |
abentley | gary_poster, adeuring: fix verified on my system. | 14:11 |
gary_poster | thanks abentley | 14:12 |
adeuring | gary_poster: r=me | 14:15 |
gary_poster | thank you adeuring! | 14:15 |
=== salgado is now known as salgado-off | ||
jpds | adeuring: Hi, did you ec2 land my branch? | 15:46 |
adeuring | jpds: sorry, forgot it... Will do that now. | 15:46 |
jml | adeuring: 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 |
adeuring | jml: sure | 15:53 |
jml | thanks. | 15:53 |
jpds | adeuring: No problem, thanks! | 15:56 |
allenap | adeuring: 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/23105 | 16:06 |
adeuring | allenap: done | 16:09 |
allenap | adeuring: Thanks :) | 16:10 |
allenap | adeuring: 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 |
adeuring | allenap: done. (sorry for the delay -- was on skype) | 16:22 |
allenap | adeuring: Thanks :) | 16:22 |
=== matsubara is now known as matsubara-lunch | ||
adeuring | jml: r=me | 16:32 |
jml | adeuring, thanks. | 16:34 |
allenap | adeuring: 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/23113 | 16:34 |
adeuring | allenap: yes | 16:35 |
adeuring | allenap: r=me | 16:46 |
allenap | adeuring: Thanks! | 16:46 |
allenap | adeuring: 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/23114 | 16:50 |
adeuring | allenap: rs=me | 16:51 |
allenap | adeuring: 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 | ||
deryck | abel is gone. Is anyone else reviewing this afternoon? | 19:45 |
deryck | rockstar 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 |
abentley | deryck, sure. | 19:52 |
deryck | abentley, thanks! https://code.edge.launchpad.net/~deryck/launchpad/no-linky-abandoned-branches-559325/+merge/23135 | 19:52 |
abentley | deryck, was there a preimplementation call? | 19:55 |
deryck | abentley, no. I just did it on a whim at lunch. | 19:56 |
maxb | deryck: I question the appropriateness of this change because it hides potential partial work done on a bug that someone else could rescue and complete | 19:56 |
abentley | deryck, I'm not sure this is the right solution. | 19:56 |
deryck | abentley, ah, ok. fair enough. I did discuss the proposed solution with jml in the bug. And he agreed with the approach. | 19:56 |
abentley | deryck, yes, I saw that. Still, he's not on the code team anymore. | 19:58 |
abentley | deryck, my reasons are much like maxb's. | 19:58 |
deryck | abentley, I didn't see maxb's comment? Where is that? | 19:59 |
abentley | deryck, 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 |
abentley | deryck, in channel, 4 minutes ago. | 19:59 |
deryck | ah, sorry. thought that was you :-) | 20:00 |
deryck | the red bled together in xchat :-) | 20:00 |
abentley | perhaps appropriate in this case. | 20:01 |
deryck | abentley, so your argument would be if you don't want the connection then unlink the branch? | 20:02 |
abentley | deryck, that seems suboptimal too. | 20:03 |
abentley | deryck, 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 |
maxb | Would it not be sufficient to display the branch's "Abandoned" status alongside its name in the bug "Related branches" list? | 20:04 |
deryck | abentley, I would guess more the latter. But I think it's hard to presume what anyone means by "abandoned" really. | 20:04 |
deryck | maxb, it's already listed. | 20:04 |
deryck | maxb, sorry, I'm wrong | 20:05 |
deryck | was thinking of rejected. | 20:05 |
maxb | MP status is shown, branch status is nort | 20:05 |
maxb | *not | 20:05 |
deryck | right | 20:05 |
deryck | yeah, maybe that's the right fix. | 20:05 |
abentley | deryck, so I think branch status should be shown, and we should maybe reduce the visibility of the merge proposal. | 20:06 |
abentley | deryck, but I'd be interested to hear what rockstar thinks. | 20:06 |
maxb | I like the idea of reducing the visibility of the MP | 20:07 |
deryck | abentley, 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 |
deryck | but I'm thinking as a bugs guy obviously :-) | 20:08 |
maxb | Drop what? the MP or the entire branch? | 20:08 |
deryck | entire branch per my fix now | 20:08 |
maxb | Just because someone has abandoned a branch does not mean that it does not contain useful work | 20:09 |
abentley | deryck, if I'm not mistaken, the reason the branch status isn't shown is because the MP *is* shown. | 20:09 |
deryck | maxb, 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 |
maxb | Better to err on the side of not losing visibility on potential useful code, IMO | 20:11 |
deryck | abentley, 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 |
deryck | maxb, true. but my assumption is someone marked it "abandoned" precisely because it wasn't useful. | 20:12 |
abentley | deryck, 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 |
abentley | deryck, 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 |
deryck | maxb, 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 |
abentley | deryck, yes, that's what it means. | 20:14 |
maxb | Well, that's what the English word means to me :-) | 20:15 |
deryck | fair 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 |
deryck | Thanks anyway, abentley, for the review. | 20:17 |
abentley | deryck, very well. Sorry it didn't work out. | 20:18 |
deryck | no worries | 20:18 |
* rockstar sees that he was apparently pinged in this channel while at lunch | 21:01 | |
abentley | rockstar, don't worry about it. | 21:05 |
rockstar | abentley, yeah, it looks like there was consensus. | 21:06 |
abentley | rockstar, well, more like resolution. | 21:06 |
rockstar | abentley, 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 |
rockstar | In that case, I'd only show the one that fixed the issue. | 21:10 |
abentley | rockstar, chat? | 21:15 |
rockstar | abentley, oh, it's getting to be that time huh? | 21:16 |
rockstar | abentley, can you give me 5 minutes to get my tests passing? | 21:16 |
abentley | rockstar, 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!