| === flacoste is now known as flacoste_afk | ||
| === flacoste_afk is now known as flacoste | ||
| rinze | wgrant: Hi | 09:22 |
|---|---|---|
| wgrant | rinze: Morning. | 09:22 |
| 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:23 |
| rinze | wgrant: Great, thanks. | 09:24 |
| === 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 | ||
| jtv | hi jelmer, nice & easy review for you: https://code.edge.launchpad.net/~jtv/launchpad/translations-lint/+merge/30633 | 11:52 |
| jtv | worth perhaps one beer next week :) | 11:53 |
| === 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 | ||
| * jelmer waves | 11:55 | |
| jtv | jelmer: I removed some unused methods as well. | 11:56 |
| jtv | Ugh... I introduced some failures in the tests I touched. Fixing. | 11:57 |
| jtv | All fixed now. | 12:00 |
| jtv | Still used to compilers or lint to catch this basic stuff I guess. | 12:01 |
| jtv | jelmer: going for lunch now, back in about an hour | 12:18 |
| jelmer | jtv: k | 12:19 |
| jelmer | jtv: Can I already have a look at the branch or do you still need to push up those test fixes? | 12:20 |
| wgrant | jelmer: Thanks. | 12:35 |
| wgrant | jelmer: I considered making the argument optional, but it was unprecedented... | 12:36 |
| 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:38 |
| jelmer | wgrant: ec2 is doing its thing | 12:41 |
| wgrant | jelmer: Thanks. | 12:41 |
| === 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 | ||
| 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 ? | 13:31 |
| === 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 | ||
| jelmer | wb adeuring, jtv | 14:09 |
| === 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 | ||
| adeuring | jelmer: wb? | 14:09 |
| jtv | jelmer: "wb"? | 14:10 |
| === 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 | ||
| jelmer | welcome back! | 14:10 |
| jtv | ah :) | 14:10 |
| adeuring | jelmer: ah, thanks :) | 14:10 |
| jtv | any luck with the reviews? | 14:10 |
| jelmer | jtv: Yep | 14:12 |
| jelmer | jtv: My only concern wrt your branch was the copyright years; when do we update them? | 14:12 |
| 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:13 |
| 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:16 |
| jtv | jelmer: wonderful, thanks. | 14:17 |
| === 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 | ||
| jelmer | adeuring: I was looking at is_security_proxied_or_harmless and wondered about the unwrapped_types. | 14:25 |
| adeuring | jelmer: yes? | 14:25 |
| 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:26 |
| 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:27 |
| jelmer | adeuring: r=me | 14:57 |
| 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:58 |
| jelmer | s/type/typo/ | 14:59 |
| adeuring | jelmer: thanks! will fix the comments | 14:59 |
| === 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 | ||
| 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:12 |
| jtv | yeah, my branch bounced | 15:13 |
| jelmer | it's the same error as this morning | 15:13 |
| jtv | :-( | 15:14 |
| jtv | I'm screaming for losas on -code | 15:14 |
| 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:34 |
| === matsubara is now known as matsubara-lunch | ||
| jelmer | jtv: Ok | 15:45 |
| jtv | jelmer: the difference won't be huge of course, so leaving it up to you. | 15:46 |
| * jelmer sees whether the branch is comprehensible enough as is | 15:47 | |
| === 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 | ||
| 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:13 |
| benji | b) should be "make a commit to the already-reviewed branch and ask for more reviewing" | 21:14 |
| 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:15 |
| benji | k, thanks | 21:16 |
| === matsubara is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!