[09:22] <rinze> wgrant: Hi
[09:22] <wgrant> rinze: Morning.
[09:23] <wgrant> 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] <rinze> wgrant: Great, thanks.
[11:52] <jtv> hi jelmer, nice & easy review for you: https://code.edge.launchpad.net/~jtv/launchpad/translations-lint/+merge/30633
[11:53] <jtv> worth perhaps one beer next week :)
[11:55]  * jelmer waves
[11:56] <jtv> jelmer: I removed some unused methods as well.
[11:57] <jtv> Ugh... I introduced some failures in the tests I touched.  Fixing.
[12:00] <jtv> All fixed now.
[12:01] <jtv> Still used to compilers or lint to catch this basic stuff I guess.
[12:18] <jtv> jelmer: going for lunch now, back in about an hour
[12:19] <jelmer> jtv: k
[12:20] <jelmer> jtv: Can I already have a look at the branch or do you still need to push up those test fixes?
[12:35] <wgrant> jelmer: Thanks.
[12:36] <wgrant> jelmer: I considered making the argument optional, but it was unprecedented...
[12:38] <wgrant> I've fixed the typo and year, though.
[12:38] <jelmer> wgrant: Do you need me to ec2 land ?
[12:38] <wgrant> jelmer: That would be wonderful.
[12:41] <jelmer> wgrant: ec2 is doing its thing
[12:41] <wgrant> jelmer: Thanks.
[13:31] <adeuring> jelmer: can I ad this mp to your queue: https://code.edge.launchpad.net/~adeuring/launchpad/security-guarded-test-object-factory-2/+merge/30642 ?
[14:09] <jelmer> wb adeuring, jtv
[14:09] <adeuring> jelmer: wb?
[14:10] <jtv> jelmer: "wb"?
[14:10] <jelmer> welcome back!
[14:10] <jtv> ah :)
[14:10] <adeuring> jelmer: ah, thanks :)
[14:10] <jtv> any luck with the reviews?
[14:12] <jelmer> jtv: Yep
[14:12] <jelmer> jtv: My only concern wrt your branch was the copyright years; when do we update them?
[14:13] <jelmer> 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] <jtv> jelmer: afaik whenever we update the source
[14:16] <jelmer> 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] <jtv> jelmer: wonderful, thanks.
[14:25] <jelmer> adeuring: I was looking at is_security_proxied_or_harmless and wondered about the unwrapped_types.
[14:25] <adeuring> jelmer: yes?
[14:26] <jelmer> 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] <adeuring> 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] <jelmer> ah, ok. So it's updated as we encounter false positives?
[14:27] <adeuring> jelmer: and there are not methods returning floats
[14:27] <adeuring> jelmer: yes, that's the idea
[14:57] <jelmer> adeuring: r=me
[14:58] <jelmer> 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] <jelmer> adeuring: I noticed test_is_security_proxied_or_harmless__sequence_harmful_content contains a type in its comment (missing "some" ?)
[14:59] <jelmer> s/type/typo/
[14:59] <adeuring> jelmer: thanks! will fix the comments
[15:12] <jelmer> jtv: looks like we're in testfix mode again
[15:12] <jelmer> I think your merge request was refused as well
[15:12] <jtv> jelmer: thanks for the notice...  I'll have a look in case it was me
[15:13] <jtv> yeah, my branch bounced
[15:13] <jelmer> it's the same error as this morning
[15:14] <jtv> :-(
[15:14] <jtv> I'm screaming for losas on -code
[15:34] <jtv> 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.
[15:45] <jelmer> jtv: Ok
[15:46] <jtv> 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
[21:13] <benji> 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] <benji> darn, I fail at multiple choice
[21:14] <benji> b) should be "make a commit to the already-reviewed branch and ask for more reviewing"
[21:15] <abentley> 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] <benji> k, thanks