=== flacoste is now known as flacoste_afk === flacoste_afk is now known as flacoste [09:22] wgrant: Hi [09:22] rinze: Morning. [09:23] rinze: https://code.edge.launchpad.net/~wgrant/launchpad/link-uploaded-ddebs/+merge/29669 is the branch for which a review would be handy. [09:24] wgrant: Great, thanks. === rinze changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [11:52] hi jelmer, nice & easy review for you: https://code.edge.launchpad.net/~jtv/launchpad/translations-lint/+merge/30633 [11:53] worth perhaps one beer next week :) === jelmer changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: wgrant || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [11:55] * jelmer waves [11:56] jelmer: I removed some unused methods as well. [11:57] Ugh... I introduced some failures in the tests I touched. Fixing. [12:00] All fixed now. [12:01] Still used to compilers or lint to catch this basic stuff I guess. [12:18] jelmer: going for lunch now, back in about an hour [12:19] jtv: k [12:20] jtv: Can I already have a look at the branch or do you still need to push up those test fixes? [12:35] jelmer: Thanks. [12:36] jelmer: I considered making the argument optional, but it was unprecedented... [12:38] I've fixed the typo and year, though. [12:38] wgrant: Do you need me to ec2 land ? [12:38] jelmer: That would be wonderful. [12:41] wgrant: ec2 is doing its thing [12:41] jelmer: Thanks. === jelmer changed the topic of #launchpad-reviews to: On call: jelmer (lunch) || reviewing: - || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mrevell is now known as mrevell-lunch [13:31] jelmer: can I ad this mp to your queue: https://code.edge.launchpad.net/~adeuring/launchpad/security-guarded-test-object-factory-2/+merge/30642 ? === adeuring changed the topic of #launchpad-reviews to: On call: jelmer (lunch) || reviewing: - || queue: [jtv, adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === mrevell-lunch is now known as mrevell [14:09] wb adeuring, jtv === jelmer changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [jtv, adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:09] jelmer: wb? [14:10] jelmer: "wb"? === jelmer changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: adeuring || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:10] welcome back! [14:10] ah :) [14:10] jelmer: ah, thanks :) [14:10] any luck with the reviews? [14:12] jtv: Yep [14:12] jtv: My only concern wrt your branch was the copyright years; when do we update them? [14:13] Some other projects I've worked on don't consider an empty line removal creative enough to warrant updating the Copyright line year [14:13] jelmer: afaik whenever we update the source [14:16] jtv: I can't find anything in the style guide about it. Anyway, I don't think this should block your branch. r=me. I'll ask about it at the next reviewer meeting. [14:17] jelmer: wonderful, thanks. === jelmer changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:25] adeuring: I was looking at is_security_proxied_or_harmless and wondered about the unwrapped_types. [14:25] jelmer: yes? [14:26] The comment describes them as simple python types, but they include for example DSCFile (which is quite complex) but exclude things like float and long. [14:27] jelmer: yes, the comment is not vrey precise. The goal of this tuple is simply to ensure that existing factory methods returning unproxied objects of "harmless" types will not emit a warning [14:27] ah, ok. So it's updated as we encounter false positives? [14:27] jelmer: and there are not methods returning floats [14:27] jelmer: yes, that's the idea [14:57] adeuring: r=me [14:58] adeuring: Since it wasn't really part of this branch I haven't commented here, but it'd be great if you could fix that comment for unwrapped_types to prevent confusion about it. [14:58] adeuring: I noticed test_is_security_proxied_or_harmless__sequence_harmful_content contains a type in its comment (missing "some" ?) [14:59] s/type/typo/ [14:59] jelmer: thanks! will fix the comments === 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 [15:12] jtv: looks like we're in testfix mode again [15:12] I think your merge request was refused as well [15:12] jelmer: thanks for the notice... I'll have a look in case it was me [15:13] yeah, my branch bounced [15:13] it's the same error as this morning [15:14] :-( [15:14] I'm screaming for losas on -code [15:34] jelmer: I have another one, but it's close to the review limit. A few of the changes should drop off the diff once the branch you just reviewed lands, so you may want to wait until then. === matsubara is now known as matsubara-lunch [15:45] jtv: Ok [15:46] jelmer: the difference won't be huge of course, so leaving it up to you. [15:47] * jelmer sees whether the branch is comprehensible enough as is === gary_poster_ is now known as gary_poster === bigjools-afk is now known as bigjools === matsubara-lunch is now known as matsubara === matsubara is now known as matsubara-afk === matsubara-afk is now known as matsubara === jelmer 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 [21:13] hello reviewer types; a question: I have a branch that was reviewed and merged but failed QA because the production OpenID provider behaves differently than the test OP, should I a) make a new branch to fix the bug and ask for that to be reviewed, b) make a new branch and a new merge proposal, or c) none of the above [21:13] darn, I fail at multiple choice [21:14] b) should be "make a commit to the already-reviewed branch and ask for more reviewing" [21:15] benji, I suggest using the existing branch and creating a new merge proposal. Though you could create a new branch if you prefer. [21:16] k, thanks === matsubara is now known as matsubara-afk