=== _thumper_ is now known as thumper === _thumper_ is now known as thumper === Ursinha is now known as Ursinha-afk [07:48] good morning adeuring! Any chance I could bother you for a review that didn't make it through the queue last night? https://code.launchpad.net/~jtv/launchpad/bug-409686/+merge/26179 [07:48] jtv: sure [07:48] adeuring: wonderful, thanks! === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: jtv || queue: [sinzui, jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [08:37] jtv: r=me [08:37] adeuring: thanks! [08:38] adeuring: good point on the LOGIN... how about USERNAME? [08:38] jtv: right, that looks much nicer! [08:38] Oh [08:38] idea [08:39] No wait, no idea. [08:39] Capital letters are the only good way I can think of there to say "this isn't literal" [08:39] I thought of using quotes, but that may be confusing because the instructions do contain a quoted string. So the quotes would have different meanings. [08:40] right. BTW, how do we handle these "placeholders" elsewhere? [08:45] Well ideally, it'd be nice if I could fill in the real username there. But the message may be going out to teams. === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: - || queue: [sinzui, jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [08:51] jtv: We know the LP user when we generate the email, don't we? So, can't we simply insert his login name? [08:52] Hmm... I'd have to iterate over the team if there was one, I guess. [08:53] How about I file a bug for it as a separate job? [08:53] jtv: Ah, OK. So, let's leave it as it is [09:02] hey danilos [09:02] jtv, hey === StevenK changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: - || queue: [sinzui, jelmer,StevenK] || 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, StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [10:21] StevenK: I can review your branch if you show me where I can find it [10:21] adeuring: Sure -- https://code.edge.launchpad.net/~stevenk/launchpad/poppy-sftp-updates/+merge/26279 [10:22] Wow. Was it that bad ... === adeuring changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: StevenK || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [10:46] StevenK: your branch screams for a few tests ;) For example, it is great that you added the missing "import FileIsADirectory" but the fact that this was not spotted earlier is a clear indication that SFTPFile.writeChunk(9 is not properly tested. [10:53] adeuring: Right, I'm writing a test for that now. [10:53] StevenK: great, thanks! [10:54] StevenK: similary, would you mind to add tests for the constructor of class Hooks? [10:57] StevenK: well, actually, you are testing that Hooks() accepts the prefix parameter, but could you add a test thats ensures that it is used in the filename? [10:59] * adeuring needs a better mental OCR algorithm [10:59] I misread "twisted" for 2test".... === matsubara-afk is now known as matsubara [13:31] adeuring: If you're still here, I'll be pushing the test fixes to my branch in a few minutes. [13:31] StevenK: sure, go ahaed [13:38] adeuring: Pushed. [13:38] * adeuring is looking [13:48] StevenK: thanks for the new tests! r=me [13:50] adeuring: Thanks for the review :-) === Ursinha-afk is now known as Ursinha [13:55] StevenK, what are the consequences of landing after the release? [14:32] sinzui: You'd prefer to have this land as a CP? [14:32] I just want to understand the urgency. [14:33] sinzui: It has a feature that Julian thinks is fairly important, but if you'd rather it was CP'd, that's okay too. [14:35] okay, you have my RC on contingent you update the XXX comment [14:36] Which one? [14:37] 51 # XXX: Eww bug=586695 [14:37] Ah. You'd prefer something more descriptive? :-) [14:40] sinzui: Done, and pushed. [14:40] thanks [14:43] sinzui: Fixed harder, based on your XXX style comment. Anyway, throwing it through ec2 [14:44] well. I have no confidence in ec2 in the moment. I think your changes are isolated so your own testing gives me more confidence. [14:45] Poor ec2. What did it ever do to deserve such .... Oh. Never mind. :-P [14:45] sinzui: I'm happy to pqm-submit, but I have not run the entire test suite over the code. [14:45] lets use ec2 [14:48] sinzui: Mind you, I share your vote of no confidence. [15:05] sinzui: i tried this morning to find an MP from you but couldn't find any [15:06] adeuring, there was confusion yesterday and my name appears to have been added back. I have no branches thanks. === sinzui changed the topic of #launchpad-reviews to: On Call: adeuring || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews [15:06] sinzui: ah, ok === StevenK 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 === salgado-afk is now known as salgado === matsubara is now known as matsubara-lunch === adeuring 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 === salgado is now known as salgado-lunch === matsubara-lunch is now known as matsubara === Ursinha is now known as Ursinha-nomnom === salgado-lunch is now known as salgado === Ursinha-nomnom is now known as Ursinha === matsubara is now known as matsubara-afk === salgado is now known as salgado-afk === Ursinha is now known as Ursinhaway