| === maxb changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [maxb(trivial-bad-httpcaller-constructor-invocation)]|| This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === maxb changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [maxb(trivial-bad-httpcaller-constructor-invocation,py2.6-warnings-monkeypatching)]|| This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| abentley | james_w, cool. | 02:27 |
|---|---|---|
| maxb | Hi, if I add an XXX comment, is it really mandatory that I file a bug, just to track the fact that the XXX comment exists? | 02:33 |
| maxb | I have difficulty understanding the rationale | 02:34 |
| abentley | maxb, the bug is tracking the fact that the underlying issue exists. | 02:37 |
| maxb | even when the 'issue' is simply 'this code jumps through extra hoops to support both karmic and lucid' ? | 02:38 |
| abentley | maxb, XXX comments don't have a lot of visibility, and tracking all our issues in one place has definite advantages. | 02:40 |
| abentley | maxb, perhaps it would help if you regarded the XXX comment as optional. | 02:40 |
| mwhudson | maxb: you could file a bug that says "there is code that jumps through extra hoops to support both karmic and lucid", and then when karmic is not supported, someone can see the bug and grep for all places that can now be simplified | 02:41 |
| mwhudson | you certainly wouldn't need one bug per XXX | 02:42 |
| abentley | maxb, but I don't think anyone has reviewed the XXX policy for a while. Maybe you'd like to bring it up at the next reviewer meeting? | 02:42 |
| maxb | I guess I can see some | 02:43 |
| maxb | oops | 02:43 |
| maxb | I guess I can see partially where your arguments are coming from. However I contrast that with things like bug 116048, which I am now fixing because I've come across the XXX in the code, and am closing the referenced bug as an afterthought | 02:44 |
| mup | Bug #116048: Remove test_ftparchive.py hack when pqm/lpnet moves to feisty <tech-debt> <test-system> <Launchpad Foundations:In Progress by maxb> <https://launchpad.net/bugs/116048> | 02:44 |
| mwhudson | maxb: come to think of it, i think the useful part of the policy is that the bug number lets me find _other_ places that can be fixed | 02:47 |
| mwhudson | this definitely happened when bzr upgrades weren't done in lock step | 02:47 |
| mwhudson | so you'd end up with code all over to work with 1.17 and 1.18 (say), then a while later you'd find one of the XXXs | 02:47 |
| mwhudson | it was nice to be able to find all the others that could be cleaned up at the same time | 02:47 |
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [maxb(trivial-bad-httpcaller-constructor-invocation,py2.6-warnings-monkeypatching),wgrant(emailauthentication.txt-2.6-fix)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === maxb changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [maxb(trivial-bad-httpcaller-constructor-invocation,py2.6-warnings-monkeypatching),wgrant(emailauthentication.txt-2.6-fix),maxb(tolerate-lucid-apt-ftparchive)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| maxb | poor OCR tomorrow morning | 03:19 |
| maxb | still, most of those are fairly trivial | 03:19 |
| abentley | mwhudson, you should also be able to use root_transport on line 35 of the preview diff. | 03:26 |
| mwhudson | abentley: oh right, i got one of them but not the other | 03:27 |
| abentley | mwhudson, r=me | 03:27 |
| mwhudson | abentley: thanks | 03:27 |
| wgrant | Hmm, I wonder if this will make the topic too long. | 06:01 |
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [maxb(trivial-bad-httpcaller-constructor-invocation,py2.6-warnings-monkeypatching),wgrant(emailauthentication.txt-2.6-fix),maxb(tolerate-lucid-apt-ftparchive),do-not-shallow-copy-os.environ] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [maxb(trivial-bad-httpcaller-constructor-invocation,py2.6-warnings-monkeypatching),wgrant(emailauthentication.txt-2.6-fix),maxb(tolerate-lucid-apt-ftparchive),wgrant(do-not-shallow-copy-os.environ)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: maxb || queue: [maxb(py2.6-warnings-monkeypatching),wgrant(emailauthentication.txt-2.6-fix),maxb(tolerate-lucid-apt-ftparchive),wgrant(do-not-shallow-copy-os.environ)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: maxb || queue: [wgrant(emailauthentication.txt-2.6-fix),maxb(tolerate-lucid-apt-ftparchive),wgrant(do-not-shallow-copy-os.environ)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: wgrant || queue: [maxb(tolerate-lucid-apt-ftparchive),wgrant(do-not-shallow-copy-os.environ)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | wgrant: you linked to the (partially) fixed issue in python on your MP, but is there another python issue for the remaining bug? | 08:09 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: maxb || queue: [wgrant(do-not-shallow-copy-os.environ)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === danilos changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: maxb || queue: [wgrant(do-not-shallow-copy-os.environ), danilo] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: wgrant || queue: [danilo,jml,sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: danilos || queue: [jml,sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| noodles775 | danilos: which branch? (it's not listed on active reviews?) | 08:35 |
| danilos | noodles775, oh, it seems I forgot to press the 'propose merge' | 08:35 |
| danilos | noodles775, anyway, I am first interested in some discussion on it | 08:35 |
| noodles775 | Great. | 08:36 |
| danilos | noodles775, https://code.edge.launchpad.net/~danilo/launchpad/bug-553093/+merge/23458 | 08:36 |
| danilos | noodles775, what I am struggling with is coming up with a decent test | 08:36 |
| danilos | noodles775, basically, POST fails because we are snapshooting entire Language object, and translators property can have more than 1000 Person objects in there | 08:37 |
| danilos | noodles775, I've crafted a start of a test (-r-2.., pasted in the MP), but because of all the interaction (I need to add karmacache entries, and then coming back from 'karma' DB user I can't get the views to be constructed properly because of some missing Zope init) | 08:39 |
| danilos | noodles775, so, considering it took ~2 minutes on my computer to get all 1001 person objects with karma constructed, I'd rather not even have a test for this | 08:39 |
| noodles775 | ! | 08:39 |
| noodles775 | (at the 2 minutes) | 08:40 |
| noodles775 | ok, I'll have a look and see if there's anything I could suggest (but guessing you've thought through most options). | 08:40 |
| danilos | noodles775, thanks | 08:45 |
| mrevell | wgrant, Thanks for mentioning Mercurial imports the other. I spoke to Jelmer and he'd rather we didn't announce them just yet. | 09:01 |
| noodles775 | danilos: sorry, just coming back to your MP. Would it make any sense to you to instead test the interface (ie. that the translators field provides IDoNotSnapshot)? | 09:10 |
| noodles775 | It's not a test reproducing the real issue, but it will ensure (and document) the issue? | 09:10 |
| noodles775 | s/ensure/ensure that the fix remains | 09:10 |
| === maxb changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: danilos || queue: [jml,sinzui,maxb(use-pygpgme-r49)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: jml || queue: [sinzui,maxb(use-pygpgme-r49)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| wgrant | noodles775: I do not know. | 09:32 |
| danilos | noodles775, sure, I can do that | 09:32 |
| danilos | noodles775, sorry (had a call and all) | 09:33 |
| noodles775 | danilos: I added another idea on the MP too. See what you think. | 09:33 |
| noodles775 | wgrant: ok. | 09:33 |
| wgrant | noodles775: I'll have a hunt now. | 09:34 |
| noodles775 | Thanks. | 09:34 |
| danilos | noodles775, I'll see how complicated your second idea is and if I get lost in it, I'll just test the interface :) | 09:34 |
| noodles775 | danilos: great. | 09:35 |
| === henninge_ is now known as henninge | ||
| === daniloff is now known as danilos | ||
| jpds | jml: Hi there, I have a small db patch that needs reviewing; when you have time. :) https://code.launchpad.net/~jpds/launchpad/mirrorprober-next-probe-schema/+merge/23467 | 10:29 |
| jml | jpds, can you please get BjornT to review it? | 10:29 |
| jpds | jml: Sure, I've reassigned the review request. | 10:31 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: sinzui || queue: [maxb(use-pygpgme-r49)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: noodles775 || reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 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 | ||
| leonardr | noodles775, need a tiny review of https://code.edge.launchpad.net/~leonardr/launchpad/test-new-lazr-restful/+merge/23470 | 11:40 |
| noodles775 | leonardr: heh, we really need a rubber-stamp for some requests... unless there is something I'm missing that I can actually review? | 11:41 |
| * noodles775 approves | 11:41 | |
| leonardr | no, it's a rubber-stamp | 11:41 |
| === wgrant changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| danilos | noodles775, btw, I've updated https://code.edge.launchpad.net/~danilo/launchpad/bug-553093/+merge/23458 (used IDoNotSnapshot); will you have a chance to take another look? | 12:55 |
| noodles775 | danilos: sure. | 13:01 |
| danilos | thanks | 13:04 |
| danilos | noodles775, I am out for some food, should be back shortly | 13:13 |
| noodles775 | danilos: I just approved, but enjoy lunch :) | 13:13 |
| === bigjools changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [wgrant, bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| bigjools | noodles775: are you on call? :) | 13:23 |
| noodles775 | bigjools: I was fo the first 4.5hrs, but due to ui reviews throughout the week, was going to finish to catch up on other work :) | 13:24 |
| bigjools | allrighty then :) | 13:25 |
| noodles775 | bigjools: but given the trivial line counts of yours and wgrants.... | 13:25 |
| bigjools | yeah mine's a 1 line fix and 1 line test change :) | 13:26 |
| noodles775 | yep, it's approved. | 13:26 |
| bigjools | fanks | 13:26 |
| === matsubara-afk is now known as matsubara | ||
| wgrant | noodles775: It passes if you run it a few times. | 14:03 |
| wgrant | Sometimes only if it's on its own. | 14:03 |
| wgrant | This is probably why they're not run by default. | 14:03 |
| noodles775 | wgrant: Yep, thanks. It's on ec2 to land. | 14:03 |
| wgrant | Thanks. | 14:04 |
| === noodles775 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 | ||
| maxb | noodles775: Hi, did you send ~maxb/launchpad/use-pygpgme-r49 to ec2? | 15:26 |
| noodles775 | maxb: yep, it's still running the tests. | 15:28 |
| intellectronica | sinzui: i've invited you to mentor a ui review i did for gmb. note that we've already agreed on some improvements which he's working on. | 15:38 |
| sinzui | fab | 15:39 |
| gmb | sinzui, intellectronica: Where would you rather have the button? With the activity log or next to the "next check" date? | 15:40 |
| intellectronica | gmb: i thought the activity log | 15:41 |
| gmb | intellectronica, I think so to - it's consistent with the code imports ui then - but I was just checking :) | 15:41 |
| intellectronica | cool | 15:41 |
| === jml changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [jml] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jml | https://code.edge.launchpad.net/~jml/launchpad/extract-ssh-server-auth/+merge/23482 fwiw | 15:46 |
| intellectronica | jml: if it's something i can fit in 15m i'm happy to volunteer. is it? | 15:46 |
| jml | intellectronica, it's almost entirely moving code around, so I think so. | 15:47 |
| intellectronica | oright, on it | 15:47 |
| jml | intellectronica, thank you | 15:47 |
| === intellectronica 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 | ||
| gmb | intellectronica, sinzui I'm tempted to say that the "Reschedule" button should actually be labelled "Update now" like it is on code. What do you think? | 15:48 |
| intellectronica | gmb: yes, that's _much_ better | 15:48 |
| gmb | Cool | 15:48 |
| sinzui | gmb, yes, the former implies I can set a date | 15:49 |
| gmb | sinzui, Exactly, that's what I was thinking. | 15:49 |
| intellectronica | jml: there are lots of places where the moved code doesn't exactly conform to our coding conventions. is that something you want to deal with at all? | 15:50 |
| intellectronica | it's fine by me if not | 15:50 |
| intellectronica | but there are bonus points if you do | 15:51 |
| jml | intellectronica, yes please | 15:51 |
| intellectronica | jml: "yes, please, tell me about all these places so that i can correct them" or "yes, please, let's skip this for now, the code was already there, and it works"? | 15:52 |
| jml | intellectronica, the former. sorry for the ambiguity. | 15:52 |
| intellectronica | jml: avatarId --> avatar_id, for example | 15:52 |
| intellectronica | i think there are similar cases | 15:52 |
| intellectronica | userDict --> user_dict | 15:53 |
| jml | intellectronica, ok. I'll clean those up. | 15:54 |
| intellectronica | jml: is StringTransportWith_setTcpKeepAlive an idiom for eventhandlers or something? | 15:54 |
| intellectronica | ok, maybe i exaggerated a bit when i said "lots". i meant "more than one". | 15:55 |
| intellectronica | ah, nm, it makes sense in a test | 15:56 |
| intellectronica | jml: everything else looks fine. r=me. | 15:57 |
| jml | intellectronica, cool, thanks. | 15:58 |
| jml | intellectronica, did you notice anything other than userDict and avatarId? | 15:59 |
| intellectronica | jml: i didn't, actually. | 15:59 |
| gmb | intellectronica, sinzui: How does this text look: https://devpad.canonical.com/~gbinns/reschedule.png ? I'm not sure whether it should be "at n of the last m attempts". | 16:02 |
| gmb | Or something else | 16:02 |
| intellectronica | gmb: this looks perfect to me. | 16:03 |
| gmb | intellectronica, Cool. | 16:03 |
| sinzui | gmb, Button label style says the words should be English title case [Update Now] | 16:03 |
| gmb | sinzui, Right, will fix that. | 16:04 |
| gmb | sinzui, intellectronica: Any other changes I need to make or is that good to go? | 16:14 |
| sinzui | gmb, intellectronicaI am happy with the changes and the feature. | 16:15 |
| gmb | Brill. | 16:16 |
| === mrevell_ is now known as mrevell | ||
| === deryck is now known as deryck[lunch] | ||
| === leonardr is now known as leonardr-afk | ||
| gmb | intellectronica, ping | 16:51 |
| intellectronica | gmb: yo | 16:51 |
| gmb | intellectronica, Hi. Are you happy for my branch to land? | 16:51 |
| intellectronica | gmb: very happy. let me mark the mp. | 16:52 |
| gmb | intellectronica, Awesome, thanks. | 16:52 |
| adiroiban | danilos: hi. Can you help me with the landing of this branch https://code.edge.launchpad.net/~adiroiban/launchpad/bug-561355/+merge/23249 or should I ask someone else? It needs an ec2 test/land. | 17:09 |
| danilos | adiroiban, sure, I can do it | 17:10 |
| adiroiban | thanks | 17:10 |
| danilos | adiroiban, thanks for working on a fix | 17:12 |
| jml | hello | 17:14 |
| jml | I have another branch to review | 17:14 |
| jml | https://code.edge.launchpad.net/~jml/launchpad/codehosting-to-services/+merge/23491 | 17:14 |
| jml | it's perhaps even simpler than my last branch. | 17:15 |
| danilos | jml, you are having too much fun coding, go strategize a bit ;) | 17:18 |
| jml | danilos, once this branch is landed, I'm done | 17:18 |
| jml | danilos, and yes, way too much fun. | 17:18 |
| === rockstar changed the topic of #launchpad-reviews to: On call: rockstar (although also packing for a trip, so latency expected) || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| rockstar | jml, all of the codehosting-to-services branch is moves right? No real changes? | 17:33 |
| jml | rockstar, pretty much. except for what I mention in my comment. | 17:33 |
| rockstar | jml, okay, so basically just docstrings and test comments got changed (outside of imports) | 17:34 |
| jml | rockstar, that sounds right. | 17:34 |
| rockstar | jml, okay. Looking now. | 17:35 |
| rockstar | jml, your comment says you had to copy keys over. Why couldn't you move them? | 17:36 |
| jml | rockstar, they are also used by the codehosting acceptance tests. | 17:36 |
| rockstar | jml, hm. Do you really think it's wise to duplicate those keys? | 17:40 |
| jml | rockstar, why would it be unwise? | 17:40 |
| rockstar | jml, duplication. It's not code, this I know. But duplication in general can become unfun. | 17:41 |
| jml | rockstar, you want me to generate a new RSA SSH key pair? | 17:41 |
| jml | rockstar, it's pretty easy. there's even a cool website that'll do it for you! | 17:41 |
| rockstar | jml, no, we can use the same one, but I'm wondering if we could symlink or something. | 17:41 |
| rockstar | jml, that sounds like a very silly website. :) | 17:41 |
| jml | rockstar, it is – http://sshkeygen.com/ | 17:42 |
| === matsubara is now known as matsubara-lunch | ||
| * rockstar groans | 17:42 | |
| jml | rockstar, anyway, I can make them symlinks if you want. it doesn't matter much either way to me | 17:44 |
| rockstar | jml, actually, it doesn't matter much to me either. The question was more or less one of Kiko's "exploratory" questions. If you're okay with it, I am. | 17:44 |
| jml | rockstar, cool. | 17:45 |
| rockstar | jml, r=me | 17:46 |
| === danilos is now known as daniloff | ||
| === deryck[lunch] is now known as deryck | ||
| jml | rockstar, thank you | 17:49 |
| rockstar | jml, no, thank you for cleaning up technical debt. | 17:50 |
| daniloff | adiroiban, just to let you know, your branch won't land tonight because someone else has broken the build; I am letting the full test suite finish just so we see if there are any other problems; out now, cheers | 17:55 |
| adiroiban | daniloff: that's ok | 18:07 |
| === gary_poster is now known as gary-lunch | ||
| * rockstar goes afk for a bit | 18:17 | |
| === matsubara-lunch is now known as matsubara | ||
| === leonardr-afk is now known as leonardr | ||
| === gary-lunch is now known as gary_poster | ||
| === matsubara is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
