/srv/irclogs.ubuntu.com/2010/08/04/#launchpad-reviews.txt

=== 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
marslifeless, 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/3120700:49
mwhudsonmars: i looked over it, but i thought the previous one was ok so ... :-)00:49
marsmwhudson, alright, could you perhaps mark this one as 'Approved' then?  I resubmitted the proposal, which wipes the slate clean.00:49
thumpermwhudson: https://code.edge.launchpad.net/~thumper/launchpad/branch-target-adapters/+merge/3169800:50
thumpermwhudson: I forgot to add the prerequisite branch on the moved errors from yesterday00:51
thumpermwhudson: as I thought it landed00:51
thumpermwhudson: but ec2 just didn't land them nor send me an email :(00:51
mwhudsonmars: looking now00:51
thumpermwhudson: it should be only 100 lines or so00:52
mwhudsonthumper: maybe you should fix that thing to let us set the prequisite branch :-)00:53
thumpermwhudson: yes, I'm going to :)00:53
* thumper goes to lunch00:54
wgrantCould 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
mwhudsonwgrant: i guess a buildd change is following on?02:23
wgrantmwhudson: I made the buildd change months ago.02:24
mwhudsonwgrant: oh ok02:25
mwhudsonwgrant: + def setBuildDebugSymbols(self, archive, build_debug_symbols):02:26
mwhudson243+ """Helper function."""02:26
mwhudsongiven that it's invoked preciselye once, and is one line long, i'm not sure how this is helping02:26
mwhudsonoh02:26
* mwhudson opens his eysys02:27
mwhudson*eyes02:27
wgrantI may have copied that from another test in that file, I think.02:27
wgrantBut it's called a couple of times.02:27
mwhudsonwgrant: you missed my 'patronizing guide to what makes a good unit test' at the epic :-)02:28
wgrantI did indeed.02:28
mwhudsonthere seem to be three tests mushed together really:02:29
mwhudsonanyone can read02:29
mwhudsonarchive owner can't set02:29
mwhudsoncommercial admin can set02:29
mwhudsonwgrant: why do you log in as the archive owner to test that anyone can read it?02:30
wgrantmwhudson: Good question.02:30
mwhudsonthumper: could you please split it into three test cases?02:39
mwhudsonerr02:39
mwhudsonwgrant: ^^02:39
mwhudsonwgrant: everything else looks fine02:39
* thumper looks up02:40
thumperhttps://code.edge.launchpad.net/~thumper/launchpad/branch-target-adapters/+merge/31698 plz?02:40
wgrantmwhudson: Thanks.02:41
wgrantWhat's the preferred test method naming convention?02:42
wgrantI've seen testFooWorks, test_fooWorks, test_foo_works...02:43
thumperI use test_<func>_extras02:43
mwhudsonwgrant: i think it's test_$thing_descriptive_words02:43
mwhudsonwgrant: if $thing is camel cased, it stays camel cased in the test name02:43
wgrantThanks.02:47
mwhudsonthumper: reviewed02:47
thumperta02:48
wgrantmwhudson: Tests split.02:57
wgrantCan you land it?02:57
wgranthttps://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
mwhudsonwgrant: no, not yet, still remembering how to configure bits and pieces03:24
wgrantmwhudson: Oh, it's that broken?03:31
wgrantlifeless: Any chance you could land the four branches above?03:31
mwhudsonwgrant: yes03:32
wgrantmwhudson: :(03:32
lifelesswgrant: mwhudson: thanks03:58
lifelesswgrant: list the branches please07:08
wgrantmwhudson: Hm, you didn't actually approve that branch that you reviewed.07:09
wgrantlifeless: 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/3160407:09
wgrantmwhudson 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
lifelesssending those tree off07:14
wgrantThanks.07:16
adeuringstub: 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 landed908:44
stubk08:52
stubadeuring: done08:59
adeuringstub: 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
jtvReview needed!  https://code.launchpad.net/~jtv/launchpad/recife-traits/+merge/3172911:51
jtvhenninge, 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
wgrantjtv-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
bacjtv-eat: may i add a simple one to your queue?  https://code.edge.launchpad.net/~bac/launchpad/progressbar_feature_switch/+merge/3174913: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
mrevelljelmer, ping14:33
jelmermrevell, hi14:33
mrevelljelmer, thanks for approving my branch :)14:34
jelmermrevell: you're welcome. are you going to publish the results from the user survey to the list at some point?14:34
mrevelljelmer, absolutely :)14:35
mrevelljelmer, I'll look through them after the release and work out a good way of using the results.14:36
bacEdwinGrubbs: ^^^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
EdwinGrubbsbac: I can review your branch. I assume that's what you're pointing at.14:47
bacyes, EdwinGrubbs14: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
abentleyEdwinGrubbs, could you please review https://code.edge.launchpad.net/~abentley/launchpad/build-duration/+merge/31655 ?16:40
EdwinGrubbsabentley: yes, please put it in the queue16: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
EdwinGrubbsbac: 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
EdwinGrubbswgrant: 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
bacEdwinGrubbs: thanks for finding that corner case.  it'll be fixed when i redo the 'official_*' properties19:07
bacEdwinGrubbs: and thanks for the review19: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
jtvEdwinGrubbs: I just did wgrant's19: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
mwhudsongary_poster: ping22:14
mwhudsongary_poster: did you see https://code.edge.launchpad.net/~mwhudson/launchpad/test_traverse-set-participation-bug-611570/+merge/31731 ?22:14
mwhudsongary_poster: sorry for only bugging you with these deep zope-type branches :-)22:14
gary_postermwhudson: did you see I approved it 26 minutes ago ;-)22:16
mwhudsongary_poster: heh, no22:16
mwhudsoni did check after i got up though...22:17
mwhudsongary_poster: thanks22:18
gary_posterwelcome, mwhudson, thanks for very nice change22:19
mwhudsongary_poster: the thing about browser:page that grated with me is that it makes zcml kinda unavoidable22:19
mwhudsonwell, not unavoidable, i proved that22:19
gary_posterheh22:19
mwhudsonbut unavoidable for sane people22:19
gary_posterso the class stuff I already mentioned.  you also meant the security stuff?22:20
mwhudsonwell22:20
mwhudsonbasically i think the metaconfigure functions should all be fairly short22:20
gary_posteroh I think I see22:20
mwhudsongary_poster: could i have taken the zope.publisher.browser.BrowserPage approach in my helper?22:21
gary_posterif 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 necessary22:21
gary_posteryes, though it would have been annoying in two ways:22:22
mwhudsongary_poster: exactly22:22
gary_poster1) you would have had to pass the class in to your helper, rather than a function22:22
gary_poster2) you still would have had to do the security dance22:23
mwhudsongary_poster: well, it's not really making configuration less necessary per se, but more equally accessible to code and zcml22:23
mwhudsongary_poster: oh ok22: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_postermore equally accessible: ack.22:24
mwhudsongary_poster: heh, not heard of __security_checker__22:24
mwhudsonanyway, 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
mwhudsonbah22: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!