=== mars 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 | ||
mars | lifeless, mwhudson, did either of you get a chance to look at my test_on_merge.py fixes branch? https://code.edge.launchpad.net/~mars/launchpad/fix-test_on_merge-578886/+merge/31207 | 00:49 |
---|---|---|
mwhudson | mars: i looked over it, but i thought the previous one was ok so ... :-) | 00:49 |
mars | mwhudson, alright, could you perhaps mark this one as 'Approved' then? I resubmitted the proposal, which wipes the slate clean. | 00:49 |
thumper | mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/branch-target-adapters/+merge/31698 | 00:50 |
thumper | mwhudson: I forgot to add the prerequisite branch on the moved errors from yesterday | 00:51 |
thumper | mwhudson: as I thought it landed | 00:51 |
thumper | mwhudson: but ec2 just didn't land them nor send me an email :( | 00:51 |
mwhudson | mars: looking now | 00:51 |
thumper | mwhudson: it should be only 100 lines or so | 00:52 |
mwhudson | thumper: maybe you should fix that thing to let us set the prequisite branch :-) | 00:53 |
thumper | mwhudson: yes, I'm going to :) | 00:53 |
* thumper goes to lunch | 00:54 | |
wgrant | Could someone please review https://code.edge.launchpad.net/~wgrant/launchpad/per-archive-build-debug-symbols/+merge/29671? It has two db reviews already. | 01:23 |
mwhudson | wgrant: i guess a buildd change is following on? | 02:23 |
wgrant | mwhudson: I made the buildd change months ago. | 02:24 |
mwhudson | wgrant: oh ok | 02:25 |
mwhudson | wgrant: + def setBuildDebugSymbols(self, archive, build_debug_symbols): | 02:26 |
mwhudson | 243+ """Helper function.""" | 02:26 |
mwhudson | given that it's invoked preciselye once, and is one line long, i'm not sure how this is helping | 02:26 |
mwhudson | oh | 02:26 |
* mwhudson opens his eysys | 02:27 | |
mwhudson | *eyes | 02:27 |
wgrant | I may have copied that from another test in that file, I think. | 02:27 |
wgrant | But it's called a couple of times. | 02:27 |
mwhudson | wgrant: you missed my 'patronizing guide to what makes a good unit test' at the epic :-) | 02:28 |
wgrant | I did indeed. | 02:28 |
mwhudson | there seem to be three tests mushed together really: | 02:29 |
mwhudson | anyone can read | 02:29 |
mwhudson | archive owner can't set | 02:29 |
mwhudson | commercial admin can set | 02:29 |
mwhudson | wgrant: why do you log in as the archive owner to test that anyone can read it? | 02:30 |
wgrant | mwhudson: Good question. | 02:30 |
mwhudson | thumper: could you please split it into three test cases? | 02:39 |
mwhudson | err | 02:39 |
mwhudson | wgrant: ^^ | 02:39 |
mwhudson | wgrant: everything else looks fine | 02:39 |
* thumper looks up | 02:40 | |
thumper | https://code.edge.launchpad.net/~thumper/launchpad/branch-target-adapters/+merge/31698 plz? | 02:40 |
wgrant | mwhudson: Thanks. | 02:41 |
wgrant | What's the preferred test method naming convention? | 02:42 |
wgrant | I've seen testFooWorks, test_fooWorks, test_foo_works... | 02:43 |
thumper | I use test_<func>_extras | 02:43 |
mwhudson | wgrant: i think it's test_$thing_descriptive_words | 02:43 |
mwhudson | wgrant: if $thing is camel cased, it stays camel cased in the test name | 02:43 |
wgrant | Thanks. | 02:47 |
mwhudson | thumper: reviewed | 02:47 |
thumper | ta | 02:48 |
wgrant | mwhudson: Tests split. | 02:57 |
wgrant | Can you land it? | 02:57 |
wgrant | https://code.edge.launchpad.net/~wgrant/launchpad/bug-605002-P-a-s-linux-any/+merge/31451, https://code.edge.launchpad.net/~wgrant/launchpad/bug-612157-ppa-quota-ddebs/+merge/31471 and https://code.edge.launchpad.net/~wgrant/launchpad/bug-241129-queue-binary-scaling/+merge/31604 would also like to be landed. | 02:57 |
mwhudson | wgrant: no, not yet, still remembering how to configure bits and pieces | 03:24 |
wgrant | mwhudson: Oh, it's that broken? | 03:31 |
wgrant | lifeless: Any chance you could land the four branches above? | 03:31 |
mwhudson | wgrant: yes | 03:32 |
wgrant | mwhudson: :( | 03:32 |
lifeless | wgrant: mwhudson: thanks | 03:58 |
lifeless | wgrant: list the branches please | 07:08 |
wgrant | mwhudson: Hm, you didn't actually approve that branch that you reviewed. | 07:09 |
wgrant | lifeless: https://code.edge.launchpad.net/~wgrant/launchpad/bug-605002-P-a-s-linux-any/+merge/31451, https://code.edge.launchpad.net/~wgrant/launchpad/bug-612157-ppa-quota-ddebs/+merge/31471, https://code.edge.launchpad.net/~wgrant/launchpad/bug-241129-queue-binary-scaling/+merge/31604 | 07:09 |
wgrant | mwhudson reviewed https://code.edge.launchpad.net/~wgrant/launchpad/per-archive-build-debug-symbols/+merge/29671, but didn't click the right buttons, apparently. | 07:10 |
lifeless | sending those tree off | 07:14 |
wgrant | Thanks. | 07:16 |
adeuring | stub: could you have a look at this mp: https://code.edge.launchpad.net/~adeuring/launchpad/bug-39674-update-retricted-flag-of-private-bugattachments/+merge/31563 ? (Robert's concerns are addressed in a different branch that already landed9 | 08:44 |
stub | k | 08:52 |
stub | adeuring: done | 08:59 |
adeuring | stub: thanks! | 09:00 |
=== jtv changed the topic of #launchpad-reviews to: On call: jtv || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
jtv | Review needed! https://code.launchpad.net/~jtv/launchpad/recife-traits/+merge/31729 | 11:51 |
jtv | henninge, are you free to review it by any chance? | 11:51 |
=== jtv is now known as jtv-eat | ||
=== salgado-afk is now known as salgado | ||
wgrant | jtv-eat: If you have time, could you please take a look at https://code.edge.launchpad.net/~wgrant/launchpad/multi-arch-builders/+merge/31740? | 12:43 |
=== mrevell is now known as mrevell-lunch | ||
=== matsubara-afk is now known as matsubara | ||
=== Ursinha-afk is now known as Ursinha | ||
bac | jtv-eat: may i add a simple one to your queue? https://code.edge.launchpad.net/~bac/launchpad/progressbar_feature_switch/+merge/31749 | 13:54 |
=== bac changed the topic of #launchpad-reviews to: On call: jtv || reviewing: - || queue: [wgrant, bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
=== mrevell-lunch is now known as mrevell | ||
=== gary_poster_ is now known as gary_poster | ||
=== benji___ is now known as benji | ||
mrevell | jelmer, ping | 14:33 |
jelmer | mrevell, hi | 14:33 |
mrevell | jelmer, thanks for approving my branch :) | 14:34 |
jelmer | mrevell: you're welcome. are you going to publish the results from the user survey to the list at some point? | 14:34 |
mrevell | jelmer, absolutely :) | 14:35 |
mrevell | jelmer, I'll look through them after the release and work out a good way of using the results. | 14:36 |
bac | EdwinGrubbs: ^^^ | 14:46 |
=== EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jtv, Edwin || reviewing: - || queue: [wgrant, bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
EdwinGrubbs | bac: I can review your branch. I assume that's what you're pointing at. | 14:47 |
bac | yes, EdwinGrubbs | 14:47 |
=== Ursinha is now known as Ursinha-lunch | ||
=== EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jtv, Edwin || reviewing: bac || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
=== matsubara is now known as matsubara-lunch | ||
abentley | EdwinGrubbs, could you please review https://code.edge.launchpad.net/~abentley/launchpad/build-duration/+merge/31655 ? | 16:40 |
EdwinGrubbs | abentley: yes, please put it in the queue | 16:40 |
=== abentley changed the topic of #launchpad-reviews to: On call: jtv, Edwin || reviewing: bac || queue: [wgrant, abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
=== salgado is now known as salgado-lunch | ||
EdwinGrubbs | bac: the feature looks good. I stumbled onto a corner case that probably should have a bug submitted for it. If the bug tracker is set as an email address, it correctly displays a checkmark for it, but the "Report a bug" link is still grayed out with a tooltip that sounds like it is unconfigured. | 17:13 |
=== Ursinha-lunch is now known as Ursinha | ||
=== deryck is now known as deryck[lunch] | ||
=== matsubara-lunch is now known as matsubara | ||
EdwinGrubbs | wgrant: is your per-archive-build-debug-symbols branch still waiting on more reviews, or did someone just forget to mark the entire mp as approved? | 17:47 |
=== salgado-lunch is now known as salgado | ||
=== deryck[lunch] is now known as deryck | ||
=== EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jtv, Edwin || reviewing: - || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
bac | EdwinGrubbs: thanks for finding that corner case. it'll be fixed when i redo the 'official_*' properties | 19:07 |
bac | EdwinGrubbs: and thanks for the review | 19:07 |
=== jtv-eat is now known as jtv | ||
=== jtv changed the topic of #launchpad-reviews to: On call: jtv, Edwin || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
jtv | EdwinGrubbs: I just did wgrant's | 19:19 |
jtv | ...and am bowing out. | 19:19 |
=== jtv changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
=== EdwinGrubbs is now known as Edwin-lunch | ||
=== Edwin-lunch is now known as EdwinGrubbs | ||
=== matsubara_ is now known as matsubara-afk | ||
mwhudson | gary_poster: ping | 22:14 |
mwhudson | gary_poster: did you see https://code.edge.launchpad.net/~mwhudson/launchpad/test_traverse-set-participation-bug-611570/+merge/31731 ? | 22:14 |
mwhudson | gary_poster: sorry for only bugging you with these deep zope-type branches :-) | 22:14 |
gary_poster | mwhudson: did you see I approved it 26 minutes ago ;-) | 22:16 |
mwhudson | gary_poster: heh, no | 22:16 |
mwhudson | i did check after i got up though... | 22:17 |
mwhudson | gary_poster: thanks | 22:18 |
gary_poster | welcome, mwhudson, thanks for very nice change | 22:19 |
mwhudson | gary_poster: the thing about browser:page that grated with me is that it makes zcml kinda unavoidable | 22:19 |
mwhudson | well, not unavoidable, i proved that | 22:19 |
gary_poster | heh | 22:19 |
mwhudson | but unavoidable for sane people | 22:19 |
gary_poster | so the class stuff I already mentioned. you also meant the security stuff? | 22:20 |
mwhudson | well | 22:20 |
mwhudson | basically i think the metaconfigure functions should all be fairly short | 22:20 |
gary_poster | oh I think I see | 22:20 |
mwhudson | gary_poster: could i have taken the zope.publisher.browser.BrowserPage approach in my helper? | 22:21 |
gary_poster | if metaconfigure functions are short, then presumably they are calling out to python that is already factored out for reuse, and so then zcml (or, more generaically, configuration) is les necessary | 22:21 |
gary_poster | yes, though it would have been annoying in two ways: | 22:22 |
mwhudson | gary_poster: exactly | 22:22 |
gary_poster | 1) you would have had to pass the class in to your helper, rather than a function | 22:22 |
gary_poster | 2) you still would have had to do the security dance | 22:23 |
mwhudson | gary_poster: well, it's not really making configuration less necessary per se, but more equally accessible to code and zcml | 22:23 |
mwhudson | gary_poster: oh ok | 22:23 |
gary_poster | (you could have done the security dance once actually, with a trick, but I thought what you did was better than the mysterious trick of using a __Security_checker__ on the class, IIRC) | 22:24 |
gary_poster | more equally accessible: ack. | 22:24 |
mwhudson | gary_poster: heh, not heard of __security_checker__ | 22:24 |
mwhudson | anyway, it works as is now, so i don't really want to touch it again :-) | 22:25 |
gary_poster | +1 :-) | 22:25 |
=== Ursinha is now known as Ursinha-afk | ||
=== Ursinha-afk is now known as Ursinha | ||
mwhudson | bah | 22:41 |
=== Ursinha is now known as Ursinha-afk | ||
=== salgado is now known as salgado-afk |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!