[13:12] <james_w> gmb: howdy, would you have a few minutes for https://code.edge.launchpad.net/~james-w/launchpad/copy-archive-package-sets/+merge/28165 ?
[13:20] <gmb> james_w, Sure
[13:30] <stub> gmb: https://code.edge.launchpad.net/~stub/launchpad/memcache/+merge/28170
[13:31] <gmb> stub, Will look at it after I've finished james_w's
[13:31] <stub> Ta.
[13:33] <bdmurray> gmb: if you could review my mp that'd be great https://code.edge.launchpad.net/~brian-murray/launchpad/bug-97633/+merge/28109
[13:34] <gmb> bdmurray, Will do
[13:34] <gmb> Merge proposals are like buses today...
[13:42]  * gmb hates at inconsistent naming styles for test* methods.
[13:44] <gmb> james_w, r=me with one minor nitpick
[13:47] <james_w> thanks gmb
[13:48] <gmb> stub, Your first test doesn't appear to test anything. Or, rather, if it failed, what would the output be?
[13:48] <stub> eh?
[13:48]  * stub has a look
[13:49] <stub> Its testing that it doesn't explode when I omit the unit
[13:50] <gmb> stub, Ah, right, I see.
[13:51] <gmb> yes, that's now actually quite obvious. Sorry
[13:51] <stub> There are no hooks to test that the specified expiry is actually what is being used, but that is the case for the existing tests (I'd need to hack things to add the calculated cache time as an HTML comment I think).
[13:52] <stub> Which I could add easily enough but might make some tests explode.
[13:54] <gmb> stub, No need to change it (maybe you could update the narrative to say that it won't explode, I guess).
[13:54] <gmb> stub, r=me anyway
[13:54] <stub> Ta.
[14:00] <stub> gmb: https://code.edge.launchpad.net/~stub/launchpad/cache-bugcomments/+merge/28177 is the follow on to that branch. Not sure if you want to review it today or punt until after an ec2 run has discovered any test fallout.
[14:01] <gmb> stub, I might as well review it today. If EC2 results necessitate large changes then you can always request a review on just those revisions.
[14:33] <gmb> bdmurray, r=me. Nice work!
[14:39] <gmb> stub, r=me on your second branch too.
[14:54] <stub> Ta.
[14:55] <stub> gmb: Don't see a tick on https://code.edge.launchpad.net/~stub/launchpad/cache-bugcomments/+merge/28177
[14:56] <gmb> stub, Oh, it's still spinning. Hang on, I'll re-do it.
[14:56] <gmb> stub, Done
[18:50] <james_w> could somebody mark https://code.edge.launchpad.net/~james-w/launchpad/copy-archive-test-refactor/+merge/28073 approved please?
[19:07] <jelmer_> james_w: sure
[19:08] <jelmer_> james_w: can you land the changes yourself?
[19:08] <james_w> yeah, just can't approve them
[19:08] <james_w> thanks
[19:09] <bac> mars: ping
[19:36] <mars> bac, pong
[19:36] <bac> hey mars.  are you reviewing today or leonard (or no one?)  :)
[19:37] <mars> bac, ah, Leonard is sick today.  My bad, I should have swapped with him
[19:37] <mars> since I'll be RM next week
[19:38] <mars> bac, do you have a branch you would like reviewed?
[19:38] <bac> mars: actually, yes, if you can
[19:38] <bac> https://code.edge.launchpad.net/~bac/launchpad/bug-162754/+merge/28227
[19:39] <bac> mars: and could you ask leonard to be sure to be OCR next week?
[19:39] <mars> bac, I'll set a calendar item for it
[19:39] <mars> and yes, I will
[19:39] <bac> sweet
[19:39] <bac> and have fun being RM!
[19:39] <mars> CHR, OCR and RM, oh my.
[19:40] <mars> bac, btw, +1 on the branch concept - I had that exact problem when I registered Geany, which uses SF and Github
[19:40] <bac> mars: yeah, it's been a wart for a while
[19:41] <bac> hope this approach makes people happy
[19:41] <mars> bac, has the text been run by mrevell?
[19:41] <bac> mars: he looked at some early screenshots.  i'll do a 'text' review with him
[19:42] <mars> ok, thanks.  These pages are critical for conveying new concepts - the text is important
[19:43] <mars> Like "what's a driver, and why do you offer me the choice of not being one?" :)
[19:43] <bac> yep.  i've requested the review
[19:43] <bac> brb
[19:43] <mars> I take it Curtis counts as the UI reviewer?
[19:56] <bac> mars: i'll get a UI review, whether curtis or another
[19:57] <mars> bac, ok. I am doing a cursory review now, not official.  I'll include my recommendations with the code review.
[19:57] <bac> thanks mars
[21:06] <EdwinGrubbs> mars: can you review https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-535430-needspackaging-timeout-part3/+merge/28240
[21:07] <mars> EdwinGrubbs, yes, please add yourself to the que
[21:07] <mars> queue
[21:54] <mars> EdwinGrubbs, looking at your patch, I realize that I do not have the knowledge necessary to review it.  I know the upper layers of Launchpad, but little of the lower.
[21:55] <EdwinGrubbs> bac: can you review https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-535430-needspackaging-timeout-part3/+merge/28240
[21:55] <bac> EdwinGrubbs: sure
[21:56] <EdwinGrubbs> thanks
[21:57] <bac> EdwinGrubbs: the SQL comment you wrote is: Whether an upstream link should be added if it does not already exist.
[21:57] <bac> EdwinGrubbs: should that "should" be a "may"?
[21:58] <EdwinGrubbs> sure, that sounds fine.
[22:05] <bac> EdwinGrubbs: done
[22:05] <EdwinGrubbs> cool