[04:02] <spiv> Want codebrowse to generate OOPS reports?  Then please review https://code.edge.launchpad.net/~spiv/launchpad/codebrowse-oops-84838/+merge/31007 :)
[04:17]  * mwhudson looks 
[04:19] <mwhudson> spiv: er
[04:19] <mwhudson> + Strictly speaking this isn't 100% correct WSGI middleware, because it
[04:19] <mwhudson> 99	+ doesn't respect the return value from start_response (the 'write'
[04:19] <mwhudson> 100	+ callable), but using that return value is deprecated and nothing in our
[04:19] <mwhudson> 101	+ codebrowse stack uses that.
[04:19] <mwhudson> loggerhead in fact uses that callable
[04:19] <spiv> mwhudson: heh
[04:19] <spiv> mwhudson: so I just noticed!
[04:19] <spiv> mwhudson: see latest push
[04:20] <spiv> mwhudson: for some reason that didn't show up until my last-minute-before-I-push cleanups :(
[04:20] <spiv> Sorry for the noise.
[04:20] <mwhudson> heh
[04:21] <spiv> There's also something funny I'm seeing with the very first request I make to the server after starting it giving a weird HTTP error, but I'm not sure that's related to this branch.
[04:25] <spiv> I'll confess to thinking this isn't the most beautiful code I've ever written :)
[04:27] <thumper> mwhudson: I have a couple of small branches if you have a few minutes
[04:27] <thumper> https://code.edge.launchpad.net/~thumper/launchpad/fix-mirror-failure-story/+merge/31010
[04:27] <mwhudson> spiv: heh heh
[04:27] <thumper> https://code.edge.launchpad.net/~thumper/launchpad/stop-factory-mirroring/+merge/31011
[04:28] <spiv> I feel so wonderfully old-school with all these mutable variables destined to be mutated by nested functions, though!
[04:29] <mwhudson> you've been wsgi-ed!
[04:29] <spiv> Possibly there's enough going on there that I can profitably refactor out an actual object.  Initially it was just the one little variable, but well, wsgi...
[04:30] <thumper> I plan one day soon to take more of a look at loggerhead
[04:30] <mwhudson> spiv: blurgh
[04:31] <mwhudson> spiv: we shouldn't return a 200 when we're reporting an error
[04:34] <spiv> mwhudson: ah ok, I wondered about that, I meant to ask
[04:34] <spiv> I'm happy to ape whatever lpnet does here, if you happen to know :)
[04:35] <spiv> I was hoping to avoid spelunking through the layers of zope publication internals more than necessary
[04:36]  * mwhudson attempts to trigger a timeout
[04:37] <mwhudson> spiv: this timeout has a http 503
[04:38] <spiv> I'm happy to change it to 500 Internal Server Error.
[04:39] <mwhudson> spiv: please
[04:39] <spiv> I guess if some IE users get a boilerplate "IE noticed that the server broke, but I won't show you any helpful details the server may have sent about that" page then that's their problem ;)
[04:40] <mwhudson> yeah, i think that's a potential issue
[04:40] <mwhudson> but i agree with your conclusion
[04:40] <mwhudson> spiv: i reviewed the branch btw
[04:42] <spiv> Thanks!
[05:17] <thumper> mwhudson: another relatively brainless review https://code.edge.launchpad.net/~thumper/launchpad/bzr-format-matching-refactoring/+merge/31014
[05:17] <thumper> mwhudson: although more thought needed than the other two :)
[05:18] <mwhudson> thumper: looking
[05:19] <mwhudson> thumper: "# The default branch format is unrecognized."
[05:19] <mwhudson> thumper: this caused mental jarring; the default branch format is 2a, surely?
[05:19] <thumper> mwhudson: ah
[05:19] <thumper> mwhudson: hmm...
[05:20] <thumper> the idea is that if we don't recognise it, it is unrecognized
[05:20] <thumper> a rename perhaps
[05:20] <thumper> so for the test methods s/default/unrecognized/
[05:21] <mwhudson> thumper: +1
[06:19] <thumper> mwhudson: last for the day... https://code.edge.launchpad.net/~thumper/launchpad/merge-directive-handling-no-request-mirror/+merge/31019
[06:21]  * mwhudson waits for the diff
[06:23] <mwhudson> thumper: done
[06:23] <thumper> mwhudson: thanks
[06:27]  * thumper kicks off 4th ec2 land run
[12:20] <jelmer> wgrant, hi
[12:20] <wgrant> jelmer: Evening.
[12:22] <jelmer> wgrant: Any particular reason for returning None rather than raising an exception in your refactor-nuf-creation branch?
[12:23] <wgrant> jelmer: Hm, not really.
[12:24] <wgrant> Not sure which exception is appropriate, though.
[12:27] <jelmer> Probably a new one. ValueError would work too, but is perhaps a bit too generic.
[12:27] <wgrant> Right. Is it worth creating a new exception for that?
[12:28] <jelmer> Personally I think so, it'd be clearer if people come across that error. I can see the argument either way though.
[12:28] <jelmer> s/it'd/it'll/
[12:42] <jelmer> bac`: hi
[13:41] <bigjools> mars: I just send an MP mail, can I add myself to your queue please
[13:45] <bac> hi jelmer
[13:46] <jelmer> bac: 'morning!
[13:46] <jelmer> bac: I was wondering if you'd seen my follow-up to https://code.edge.launchpad.net/~jelmer/launchpad/600153-qafix/+merge/30790
[13:47] <bac> jelmer: done
[13:47] <bac> thanks for the reminder
[13:48] <jelmer> bac: thanks for the review :-)
[14:17]  * flacoste is back.
[15:31] <jtv> hi mars, thanks for reviewing me
[15:31] <mars> np
[15:52] <gary_poster> benji: I'll see how far I get.  To start:
[15:53] <gary_poster> - I'd suggest making a local release of this release of batchnavigator and integrate it with Launchpad.  Then you can see if the change to the endNumber behavior is problematic
[15:54] <gary_poster> - in lines 55 and 59 of the diff, why do you call len(batch) outside of the assertions?
[15:55] <benji> re. local release: good idea
[15:56] <gary_poster> - I don't see the value of the behavior described in lines 68-74 of the diff
[15:56] <gary_poster> shouldn't prevBatch try again, or at least puke?
[15:56] <gary_poster> as it is, it seems like an odd edge case designed to bite people unexpectedly :-)
[15:57] <gary_poster> - I like the new endNumber behavior.  I also like the new comments.
[15:57] <benji> re. len: that's cruft that shouldn't be there; removing
[15:58] <gary_poster> cool
[15:58] <gary_poster> also cruft: line 115 (#print "_get_length called (EXPENSIVE)")
[15:59] <benji> re. 68-74: me neither, I just added comments that describe what is being testsed; I don't think the behavior is useful, but I don't believe that strongly enough to remove the tests
[15:59] <gary_poster> - line 120: the Lazy decorator is supposed to mean that you just "return self._get_length(self.list)"
[15:59] <gary_poster> you shouldn't have to also set the _listlength value
[16:00] <gary_poster> 68-74: but...that's new test for this branch
[16:00] <gary_poster> maybe this is something that leonard added for this branch?
[16:00] <gary_poster> if so, my question goes to him
[16:00] <benji> the reason we set _listlength is so we can use it if available; avoiding calling listlength if not
[16:01] <benji> let me look at 68-74 again
[16:02] <gary_poster> _listlength: ah ok.  Maybe remove the @Lazy then, since I suspect it adds processing cost for the reader for little to no advantage for speed?
[16:02] <benji> gary_poster: right, Leonard added the test, I added the comment
[16:03] <gary_poster> ok.  do you want me to ping him, or do you want to pursue on your own so you feel you understand it first?
[16:03] <gary_poster> (where "pursue on your own" == "you ping Leonard" in my book, but as you wish :-) )
[16:03] <benji> re. _listlength: +1
[16:03] <gary_poster> cool
[16:03] <benji> I'll pursue
[16:04] <gary_poster> cool thanks
[16:05] <gary_poster> I don't understand "return len(self._sliced_list) - 1" yet: why -1?  Maybe I'll see soon...
[16:05] <gary_poster> (line 139)
[16:08] <gary_poster> - another cruft but: #print "sliced_list called (EXPENSIVE)" (line 206)
[16:08] <gary_poster> s/but/bit/
[16:42] <henninge> mars: Hi, I added myself to the queue and it would be great if you could get to my branch but I will also have to leave in 30 min. So I don't mind it waiting around till tomorrow, either.
[16:43] <henninge> ;-)
[16:44] <henninge> oops, confilcts ...