=== 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
[00:49] <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:50] <thumper> mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/branch-target-adapters/+merge/31698
[00:51] <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:52] <thumper> mwhudson: it should be only 100 lines or so
[00:53] <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:54]  * thumper goes to lunch
[01:23] <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.
[02:23] <mwhudson> wgrant: i guess a buildd change is following on?
[02:24] <wgrant> mwhudson: I made the buildd change months ago.
[02:25] <mwhudson> wgrant: oh ok
[02:26] <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:27]  * 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:28] <mwhudson> wgrant: you missed my 'patronizing guide to what makes a good unit test' at the epic :-)
[02:28] <wgrant> I did indeed.
[02:29] <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:30] <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:39] <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:40]  * thumper looks up
[02:40] <thumper> https://code.edge.launchpad.net/~thumper/launchpad/branch-target-adapters/+merge/31698 plz?
[02:41] <wgrant> mwhudson: Thanks.
[02:42] <wgrant> What's the preferred test method naming convention?
[02:43] <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:47] <wgrant> Thanks.
[02:47] <mwhudson> thumper: reviewed
[02:48] <thumper> ta
[02:57] <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.
[03:24] <mwhudson> wgrant: no, not yet, still remembering how to configure bits and pieces
[03:31] <wgrant> mwhudson: Oh, it's that broken?
[03:31] <wgrant> lifeless: Any chance you could land the four branches above?
[03:32] <mwhudson> wgrant: yes
[03:32] <wgrant> mwhudson: :(
[03:58] <lifeless> wgrant: mwhudson: thanks
[07:08] <lifeless> wgrant: list the branches please
[07:09] <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:10] <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:14] <lifeless> sending those tree off
[07:16] <wgrant> Thanks.
[08:44] <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:52] <stub> k
[08:59] <stub> adeuring: done
[09:00] <adeuring> stub: thanks!
=== 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
[11:51] <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?
=== jtv is now known as jtv-eat
=== salgado-afk is now known as salgado
[12:43] <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?
=== mrevell is now known as mrevell-lunch
=== matsubara-afk is now known as matsubara
=== Ursinha-afk is now known as Ursinha
[13:54] <bac> jtv-eat: may i add a simple one to your queue?  https://code.edge.launchpad.net/~bac/launchpad/progressbar_feature_switch/+merge/31749
=== 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
[14:33] <mrevell> jelmer, ping
[14:33] <jelmer> mrevell, hi
[14:34] <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:35] <mrevell> jelmer, absolutely :)
[14:36] <mrevell> jelmer, I'll look through them after the release and work out a good way of using the results.
[14:46] <bac> EdwinGrubbs: ^^^
=== 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
[14:47] <EdwinGrubbs> bac: I can review your branch. I assume that's what you're pointing at.
[14:47] <bac> yes, EdwinGrubbs
=== 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
[16:40] <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
=== 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
[17:13] <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.
=== Ursinha-lunch is now known as Ursinha
=== deryck is now known as deryck[lunch]
=== matsubara-lunch is now known as matsubara
[17:47] <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?
=== 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
[19:07] <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
=== 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
[19:19] <jtv> EdwinGrubbs: I just did wgrant's
[19:19] <jtv> ...and am bowing out.
=== 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
[22:14] <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:16] <gary_poster> mwhudson: did you see I approved it 26 minutes ago ;-)
[22:16] <mwhudson> gary_poster: heh, no
[22:17] <mwhudson> i did check after i got up though...
[22:18] <mwhudson> gary_poster: thanks
[22:19] <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:20] <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:21] <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:22] <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:23] <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:24] <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:25] <mwhudson> anyway, it works as is now, so i don't really want to touch it again :-)
[22:25] <gary_poster> +1 :-)
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha
[22:41] <mwhudson> bah
=== Ursinha is now known as Ursinha-afk
=== salgado is now known as salgado-afk