| spiv | Want codebrowse to generate OOPS reports? Then please review https://code.edge.launchpad.net/~spiv/launchpad/codebrowse-oops-84838/+merge/31007 :) | 04:02 |
|---|---|---|
| * mwhudson looks | 04:17 | |
| 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:19 |
| 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:20 |
| 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:21 |
| spiv | I'll confess to thinking this isn't the most beautiful code I've ever written :) | 04:25 |
| 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:27 |
| spiv | I feel so wonderfully old-school with all these mutable variables destined to be mutated by nested functions, though! | 04:28 |
| 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:29 |
| thumper | I plan one day soon to take more of a look at loggerhead | 04:30 |
| mwhudson | spiv: blurgh | 04:30 |
| mwhudson | spiv: we shouldn't return a 200 when we're reporting an error | 04:31 |
| 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:34 |
| spiv | I was hoping to avoid spelunking through the layers of zope publication internals more than necessary | 04:35 |
| * mwhudson attempts to trigger a timeout | 04:36 | |
| mwhudson | spiv: this timeout has a http 503 | 04:37 |
| spiv | I'm happy to change it to 500 Internal Server Error. | 04:38 |
| 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:39 |
| 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:40 |
| spiv | Thanks! | 04:42 |
| 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:17 |
| mwhudson | thumper: looking | 05:18 |
| 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:19 |
| 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:20 |
| mwhudson | thumper: +1 | 05:21 |
| thumper | mwhudson: last for the day... https://code.edge.launchpad.net/~thumper/launchpad/merge-directive-handling-no-request-mirror/+merge/31019 | 06:19 |
| * mwhudson waits for the diff | 06:21 | |
| mwhudson | thumper: done | 06:23 |
| thumper | mwhudson: thanks | 06:23 |
| * thumper kicks off 4th ec2 land run | 06:27 | |
| === bigjools is now known as bigjools-afk | ||
| === bigjools-afk is now known as bigjools | ||
| jelmer | wgrant, hi | 12:20 |
| wgrant | jelmer: Evening. | 12:20 |
| jelmer | wgrant: Any particular reason for returning None rather than raising an exception in your refactor-nuf-creation branch? | 12:22 |
| wgrant | jelmer: Hm, not really. | 12:23 |
| wgrant | Not sure which exception is appropriate, though. | 12:24 |
| 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:27 |
| 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:28 |
| jelmer | bac`: hi | 12:42 |
| === jtv changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [jtv²] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: - || queue: [jtv²] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| bigjools | mars: I just send an MP mail, can I add myself to your queue please | 13:41 |
| === bigjools changed the topic of #launchpad-reviews to: On call: mars || reviewing: - || queue: [jtv², bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === bac` is now known as bac | ||
| bac | hi jelmer | 13:45 |
| 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:46 |
| bac | jelmer: done | 13:47 |
| bac | thanks for the reminder | 13:47 |
| jelmer | bac: thanks for the review :-) | 13:48 |
| * flacoste is back. | 14:17 | |
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: jtv || queue: [bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | hi mars, thanks for reviewing me | 15:31 |
| mars | np | 15:31 |
| gary_poster | benji: I'll see how far I get. To start: | 15:52 |
| 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:53 |
| gary_poster | - in lines 55 and 59 of the diff, why do you call len(batch) outside of the assertions? | 15:54 |
| benji | re. local release: good idea | 15:55 |
| 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:56 |
| 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:57 |
| gary_poster | cool | 15:58 |
| gary_poster | also cruft: line 115 (#print "_get_length called (EXPENSIVE)") | 15:58 |
| 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 | 15:59 |
| 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:00 |
| benji | let me look at 68-74 again | 16:01 |
| 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:02 |
| 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:03 |
| gary_poster | cool thanks | 16:04 |
| 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:05 |
| gary_poster | - another cruft but: #print "sliced_list called (EXPENSIVE)" (line 206) | 16:08 |
| gary_poster | s/but/bit/ | 16:08 |
| === henninge changed the topic of #launchpad-reviews to: On call: mars || reviewing: jtv || queue: [bigjools, henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| 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:42 |
| henninge | ;-) | 16:43 |
| henninge | oops, confilcts ... | 16:44 |
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: bigjools || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: lunch || queue: [henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: henninge || queue: [lunch] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: henninge || queue: [lunch] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === jml` is now known as jml | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === mars changed the topic of #launchpad-reviews to: On call: mars || reviewing: danilos || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === 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 | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!