=== gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
lifeless | gmb: hi | 09:55 |
---|---|---|
lifeless | since you're here... | 09:55 |
gmb | Morning lifeless. What can I do for you? | 09:55 |
lifeless | your timeout bug for prod | 09:55 |
lifeless | is it ready to CP ? | 09:55 |
gmb | lifeless, I believe so; let me check... | 09:56 |
lifeless | have you seen the hourly oops graph :P | 09:57 |
gmb | lifeless, So yes, it's been merged, but into db-devel (I must've chosen the wrong target for the mp), which is annoying. So: | 10:00 |
gmb | 1) I need to prepare a branch to merge it into devel | 10:00 |
lifeless | yes, yes you do :) | 10:00 |
gmb | 2) I need to do the same for production-stable | 10:00 |
gmb | (Or whatever our CP target is these days) | 10:00 |
lifeless | have you checked on staging too see if its faster ? | 10:01 |
lifeless | thats the really interesting thing for me | 10:01 |
gmb | lifeless, It's not on staging yet. We'll have to cowboy it there. I'll grab mthaddon in a few minutes and get that done. | 10:01 |
lifeless | \o/ | 10:02 |
gmb | lifeless, Oh, hang on. It *is* in devel. I guess LP gets confused by the auto-merge from devel to db-devel. | 10:08 |
gmb | So, that's a win. | 10:08 |
jml | review wanted: https://code.edge.launchpad.net/~jml/launchpad/remove-interface-import/+merge/30375 | 11:56 |
=== leonardr changed the topic of #launchpad-reviews to: On call: gmb, leonardr || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
leonardr | jml, i got it | 11:59 |
jml | leonardr, thanks. | 12:00 |
* jml off to lunch. back soon. | 12:00 | |
jtv | gmb, leonardr: time for this one? Just extracting something to a file of its own. https://code.edge.launchpad.net/~jtv/launchpad/xpi-dtd-parser/+merge/30383 | 12:07 |
=== leonardr changed the topic of #launchpad-reviews to: On call: gmb, leonardr || reviewing: jml, jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
leonardr | jtv, i'm here all day | 12:08 |
jtv | leonardr: great, thanks | 12:08 |
=== mrevell is now known as mrevell-lunch | ||
=== leonardr changed the topic of #launchpad-reviews to: On call: gmb, leonardr || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
leonardr | jtv, just to satisfy my curiosity, what do we use the xpi dtd parser for? | 12:33 |
=== leonardr changed the topic of #launchpad-reviews to: On call: gmb, leonardr || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.om/ || https://code.launchpad.net/launchpad/+activereviews | ||
leonardr | jtv, r=me | 13:06 |
=== mrevell-lunch is now known as mrevell | ||
jtv | leonardr: thanks for the review | 13:21 |
leonardr | np | 13:21 |
rockstar | Anyone around for a review? | 13:55 |
rockstar | gmb, leonardr? | 13:55 |
lifeless | rockstar: yo | 13:56 |
rockstar | lifeless, you want to do a review? | 13:57 |
lifeless | I've done 25 or so in the last 24 hours | 13:58 |
rockstar | lifeless, https://code.edge.launchpad.net/~rockstar/launchpad/unsupported-series-recipe-api/+merge/30397 | 13:58 |
lifeless | so why not :P | 13:58 |
lifeless | rockstar: check the +activereviews page :) | 13:58 |
rockstar | lifeless, that's funny, because I've been thinking of changing the way I do reviews. | 13:59 |
lifeless | is 400 the right error to return ? | 13:59 |
rockstar | lifeless, that's what everything else returns. | 14:00 |
lifeless | oh! | 14:00 |
* lifeless adds that to the list of things to look at | 14:00 | |
rockstar | I don't even know what 400 is. | 14:00 |
rockstar | Ah, Bad Request, I think that's right. | 14:00 |
lifeless | 'The request could not be understood by the server due to malformed syntax. The client SHOULD NOT repeat the request without modifications' | 14:00 |
lifeless | its not malformed syntax | 14:00 |
rockstar | lifeless, hm, maybe. I guess REST does try and re-use HTTP descriptions for itself, so I can see the reasoning there. | 14:01 |
lifeless | 409/412 maybbe | 14:02 |
lifeless | anyhow | 14:02 |
lifeless | you're following the pattern | 14:02 |
lifeless | thats fine | 14:02 |
rockstar | 412 makes more sense than 409 | 14:03 |
rockstar | Also, it's funny that if I google for "http 40x" the fourth item returned is xkcd... | 14:03 |
jtv | gmb, leonardr: I need a taker for a cleanup branch that's over twice the review limit. Any volunteers? | 15:02 |
jtv | https://code.edge.launchpad.net/~jtv/launchpad/test-cruft-soyuz/+merge/30407 | 15:02 |
leonardr | jtv, i'll look at it | 15:02 |
jtv | Great, thanks. It's basically lint. | 15:02 |
jtv | Just lots of it. | 15:02 |
rockstar | jtv, out of curiosity, why didn't you do them in small increments? | 15:02 |
rockstar | (I've been thinking about proposing a lower limit to review diff sizes, so I'm trying to find out the cause of outliers) | 15:03 |
jtv | rockstar: it went rather faster than I expected—first you remove some boilerplate from the tests, then you run "make lint"... | 15:03 |
jtv | ...and suddenly you're at 1,400 lines & growing. | 15:03 |
rockstar | jtv, have you used bzr-pipes at all? | 15:03 |
jtv | This is rather atypical I hope. | 15:03 |
rockstar | jtv, yeah, but I think what sinzui talked about in his lightning talk is probably also the secret to my small amount of success. | 15:04 |
jtv | Once or twice, but I was scared off by two things: one was some kind of version skew that broke the plugin for me locally, and the other was that I need to look up how it works every time—and then Aaron tells me what I find in the documentation isn't actually how it should be done. | 15:04 |
rockstar | So maybe saying "No branch more than 500 lines of diff" would be helpful for everyone's velocity. | 15:04 |
sinzui | That is what U1 did | 15:05 |
rockstar | jtv, I'm happy to sit down with you and show you how I do it. It's a lot easier than one might think. | 15:05 |
rockstar | sinzui, really? That's interesting. | 15:05 |
sinzui | barry fixed his productivity by timeboxing. | 15:05 |
rockstar | sinzui, I'm curious to know what you think about my idea. | 15:05 |
jtv | rockstar: I know—I've done it. But I don't do it _quite_ frequently enough to know that I won't screw it up next time. If you could go over the online docs and make sure they're correct, that would be more valuable IMHO | 15:06 |
rockstar | jtv, yeah, that's probably a good idea. | 15:06 |
jtv | I think the limit of 800 was very helfpul, though sometimes you do just have to go over. | 15:06 |
rockstar | jtv, yeah, I'm curious if those "you just have to" situations really could be avoided more if we planned for it. | 15:06 |
sinzui | 2/4/6 hours should be enough to produce something valuable. also. I have take 2 hours to make 2000 lines of change moving something to lp.registry. The time was the correct limit | 15:07 |
rockstar | sinzui, yeah, although I'd contend that 6 hours might be a little high for most cases. | 15:07 |
rockstar | I divide my days up into 3 hour increments. | 15:07 |
sinzui | I use 2 hours, and I get nervous when I reach 4. I start looking for ways to end the branch immediately | 15:08 |
rockstar | sinzui, yeah, but I've noticed that if I'm doing anything that might touch soyuz stuff, it takes a bit longer, because I don't know all of their gotchas just yet. | 15:09 |
rockstar | (I know all the code gotchas, and how to avoid them) | 15:09 |
bigjools | rockstar: *we* don't know all of the gotchas yet | 15:09 |
sinzui | rockstar, I factor that into getting something ready to code, and I use that time to refactor code so that it is clear. tech-debt amounts to a lot of bugs in Lp. | 15:10 |
* bigjools is going to have a 1200 line branch soon. No way around it. | 15:11 | |
rockstar | sinzui, huh. That's interesting as well. A prep branch to go before the actual branch. | 15:11 |
sinzui | Salgado does a lot of them. He seems to use it to make the real task easier to do, or at least to remove any distractions from his primary task | 15:12 |
rockstar | sinzui, yeah, it's the distraction thing that annoys me. | 15:14 |
rockstar | sinzui, I've been performing an experiment with my branch workflow, and my biggest patch is ~450 lines, so I think we really can break them up more. | 15:14 |
leonardr | jtv, does lint really complain that there should be a space in ('foo',) ? | 15:15 |
leonardr | i never knew that | 15:15 |
jtv | leonardr: yup... I discussed with sinzui who confirmed that PEP8 wants it that way. | 15:15 |
jtv | (it surprised me as well :-) | 15:15 |
sinzui | Maybe this is a conversation for the review's meeting. smaller branches or shorter development time. | 15:15 |
rockstar | jtv, I've ignored that (it's stupid...) | 15:15 |
sinzui | rockstar, you should submit a patch to pep8.py them | 15:16 |
rockstar | sinzui, yeah, before I brought it to the reviewer's meeting, I wanted my own experiment. | 15:16 |
sinzui | then | 15:16 |
rockstar | Usually, the reviewer's meeting can turn into yak shaving/bikeshedding, so I wanted actual facts (similar to your lightning talk) | 15:17 |
leonardr | jtv, the blank line on line 627 of your patch looks strange. is it correct? | 15:27 |
jtv | leonardr: looking... | 15:28 |
leonardr | jtv: ditto for line 811 | 15:29 |
jtv | leonardr: yeah... those are how "make lint" wants them. | 15:29 |
leonardr | jtv: blank line before any function definition? | 15:30 |
jtv | Yes, looks like. | 15:30 |
leonardr | jtv, r=me | 15:32 |
jtv | leonardr: thanks! I'll try to bother gmb for the next one, because this just ain't fair. :-) | 15:32 |
gmb | Yes. | 15:33 |
=== deryck is now known as deryck[lunch] | ||
leonardr | jtv, for the record i got some test failures when i ran the soyuz tests, but they kind of look like a problem with my larger setup. i can paste them if you want | 15:47 |
jtv | leonardr: if they're MissingCelebrityErrors, that's a devel change and "make schema" will help. | 15:48 |
leonardr | jtv: no, it started as a GpgmeError ("unknown error code") and then cascaded from there | 15:48 |
jtv | leonardr: I think I had those a bit longer ago, and rocketfuel-{setup,get} or something made it go away. | 15:49 |
jtv | Do you get them in devel? | 15:49 |
leonardr | jtv: i don't know. sorry--i'm fighting another fire and don't want to context-switch unless this is really important to you | 15:49 |
jtv | leonardr: not particularly—ec2 will be my judge | 15:50 |
leonardr | ok | 15:50 |
jtv | gmb: here's one I'd like to ask your review for: https://code.edge.launchpad.net/~jtv/launchpad/exportstorage-cleanup/+merge/30418 | 15:50 |
gmb | jtv, Righto | 15:52 |
jtv | fab | 15:52 |
=== gmb changed the topic of #launchpad-reviews to: On call: gmb, leonardr || reviewing: jtv, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
gmb | jtv, Is the VWS on line 61 really necessary? | 15:56 |
jtv | gmb: make lint says so | 15:56 |
gmb | jtv, Garh. | 15:59 |
gmb | Okay. | 15:59 |
gmb | I'm not wholly sure the new linter isn't a bit crackful. | 15:59 |
jtv | It's a lot stricter—which by and large is a good thing, I think | 15:59 |
gmb | jtv, True. | 16:00 |
gmb | jtv, Anyway, r=me.\ | 16:00 |
jtv | \o/ thanks | 16:00 |
=== gmb changed the topic of #launchpad-reviews to: On call: gmb, leonardr || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
=== gmb changed the topic of #launchpad-reviews to: On call: leonardr || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
=== deryck[lunch] is now known as deryck | ||
jtv | leonardr: it's still highly unfair that your co-reviewer buggered off before I could be as demanding with him as I was with you... got room for a few more? | 19:15 |
jtv | I have one small lint cleanup: https://code.edge.launchpad.net/~jtv/launchpad/pre-export-cleanup/+merge/30445 | 19:15 |
leonardr | jtv, i see a conflict in potemplate.py | 19:17 |
jtv | leonardr: thanks... I'll update my branch & re-push. Hang on. | 19:17 |
jtv | leonardr: to keep you updated: I pushed a while ago but am still polling for the diff to update. Will let you know. | 19:22 |
jtv | leonardr: it's updated. | 19:23 |
leonardr | jtv, r=me | 19:28 |
jtv | leonardr: cool, thanks | 19:28 |
=== leonardr 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 | ||
* leonardr will be back in a couple hours in case no one else wants to do reviews | 19:28 | |
jtv | I'll ask around. Thanks for all the reviews today! | 19:29 |
=== danilos is now known as daniloff | ||
=== leonardr is now known as leonardr-afk | ||
=== leonardr-afk is now known as leonardr | ||
=== flacoste is now known as flacoste_afk |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!