| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| rockstar | thumper, care to do a review for me? | 02:42 |
|---|---|---|
| rockstar | thumper, https://code.edge.launchpad.net/~rockstar/lazr-js/overlay-width/+merge/33478 | 02:43 |
| thumper | rockstar: my head is down right now, how big is it? | 02:43 |
| rockstar | thumper, not big, but it adds value... | 02:43 |
| rockstar | They're merely CSS changes that fix two bugs. | 02:44 |
| === Ursinha is now known as Ursinha-afk | ||
| rockstar | Also, it'll make it easier to animate the opening and closing of the overlay. | 02:44 |
| * thumper looks | 02:45 | |
| === Ursinha-afk is now known as Ursinha | ||
| thumper | done | 02:46 |
| === Ursinha is now known as Ursinha-afk | ||
| thumper | rockstar: you around? | 03:22 |
| rockstar | thumper, indeed. | 03:22 |
| thumper | rockstar: I have a very large but boring review | 03:22 |
| rockstar | thumper, :( | 03:22 |
| rockstar | thumper, send it on over. I'll take a look at it in a sec. | 03:22 |
| thumper | rockstar: it adds layer to our browser zcml | 03:22 |
| thumper | all of the changes are either adding the layer in the zcml | 03:22 |
| thumper | or changing tests to show code.lp.dev instead of lp.dev | 03:23 |
| rockstar | (I'm currently providing marriage counseling for bzr and savannah) | 03:23 |
| thumper | or adding rootsite= to canonical url sites | 03:23 |
| thumper | heh | 03:23 |
| thumper | rockstar: https://code.edge.launchpad.net/~thumper/launchpad/fix-browser-zcml/+merge/33482 | 03:28 |
| mwhudson | 7 >>> canonical_url(getUtility(IBazaarApplication)) | 03:30 |
| mwhudson | 8- u'http://launchpad.dev/+code' | 03:30 |
| mwhudson | 9+ u'http://code.launchpad.dev/+code' | 03:30 |
| mwhudson | the +code there is freaking odd really | 03:30 |
| mwhudson | i wonder if it would actually be better to have the root objects be different for each publication | 03:30 |
| thumper | mwhudson: probably | 03:35 |
| thumper | mwhudson: but somewhat out of scope for this change | 03:35 |
| rockstar | thumper, holy balls that's a big diff... | 03:35 |
| mwhudson | thumper: yes | 03:35 |
| thumper | rockstar: but boring | 03:36 |
| rockstar | thumper, might I make a suggestion for the next time? | 03:36 |
| thumper | yes | 03:36 |
| thumper | rockstar: don't do them all at once? | 03:36 |
| rockstar | thumper, yes, for the love of Odin, yes. | 03:37 |
| thumper | :) | 03:37 |
| rockstar | thumper, here's an example: line 1640-1641 of the diff doesn't look to be along the same lines as the rest of the changes in the branch. | 03:39 |
| rockstar | Actually, 1635-1665... | 03:40 |
| thumper | rockstar: :) | 03:40 |
| thumper | yes, I removed a chunk of code that was showing bad data | 03:41 |
| thumper | the person that wrote it thought it was showing the number of new revisions this month for that branch | 03:41 |
| thumper | but what it was showing was the number of revisions on all branches this month | 03:41 |
| rockstar | thumper, the change makes sense. I just think it would be better to not get lumped in with something that's supposed to be just technical debt. | 03:42 |
| rockstar | Is there a bug filed for that change? | 03:42 |
| thumper | no | 03:45 |
| rockstar | thumper, I think there should be. | 03:46 |
| * thumper nods | 03:46 | |
| rockstar | thumper, other than that, r=me. | 03:47 |
| thumper | rockstar: thanks | 03:48 |
| rockstar | thumper, no prob. | 03:48 |
| rockstar | thumper, do you think it's a sensible suggestion to break up branches like that into smaller branches? | 03:48 |
| thumper | rockstar: yeah, probably | 03:48 |
| thumper | in hindsight I shouldn't have tried to do all at once | 03:49 |
| rockstar | thumper, do you think a 400 line diff limit would have been reasonable in breaking them up? | 03:51 |
| rockstar | (I ask because I'm planning on proposing a smaller diff limit in the next reviewers meeting) | 03:52 |
| === Ursinha-afk is now known as Ursinha | ||
| thumper | perhaps 500... | 04:15 |
| lifeless | I don't think smaller diffs are really a great need | 04:45 |
| lifeless | I think more focused work is great | 04:46 |
| lifeless | the -big- thing we had in the path was huge entire projects landing | 04:46 |
| lifeless | s/path/past/ | 04:46 |
| lifeless | ymmv | 04:46 |
| === Ursinha is now known as Jorjao | ||
| === Jorjao is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
| jtv | stub: care to review a branch for me? It's a blocking risk, but not large. https://code.edge.launchpad.net/~jtv/launchpad/recife-pre-resetCurrentTranslation/+merge/33490 | 07:32 |
| stub | jtv: Not a blocking risk yet - nothing is using it apart from tests | 07:35 |
| jtv | stub: nothing I can _land_ now, no. Hence: blocking. | 07:35 |
| stub | You have an XXX that doesn't cite a bug. | 07:36 |
| stub | There are two ways of fixing that. | 07:36 |
| jtv | stub: our policy is currently not to file bugs too far in advance. It's listed in various places though. | 07:36 |
| stub | Our policy is also no XXX's without cited bugs IIRC. So remove three letters or create a bug I think are the options. | 07:37 |
| jtv | Okay, okay, I'm removing the XXX. | 07:37 |
| jtv | Pushing. | 07:38 |
| stub | jtv: IPOFile doesn't describe the parameters, which is minor but doesn't hurt to mention translator remains unchanged and timestamp is now by default. | 07:40 |
| jtv | Pushed. | 07:40 |
| stub | Otherwise all fine. | 07:40 |
| jtv | stub: thanks. I'll update the docstring as you say. | 07:41 |
| jtv | Done. | 07:42 |
| henninge | jtv: sure, I can ;-) | 07:51 |
| jtv | ok, mp'ing now... | 07:51 |
| jtv | henninge: https://code.edge.launchpad.net/~jtv/launchpad/recife-retire-traits-helpers/+merge/33491 | 07:52 |
| henninge | jtv: r=me but please hold back landing until we have that ec2 failure figured out. | 07:57 |
| jtv | henninge: ok, thanks | 07:58 |
| jtv | henninge: I did just land another small branch on recife | 07:59 |
| henninge | oh well ... | 08:00 |
| jtv | adeuring, maybe you would like to review this very small and hopefully fun branch? https://code.edge.launchpad.net/~jtv/launchpad/fakelibrarian-commit/+merge/33322 | 08:28 |
| adeuring | jtv: sure, I'll look | 08:28 |
| lifeless | (I suggested the approach, I think, if that helps) | 08:28 |
| jtv | adeuring: it allows (some) tests that need the librarian to get by without database commits | 08:28 |
| jtv | lifeless: I ended up going with the "just slap a method on the fakelibrarian class" approach. Not exactly future-proof but effective for the time being. | 08:29 |
| lifeless | jtv: I hear we can change code in the future ;) | 08:30 |
| jtv | The future's just crazy. | 08:30 |
| adeuring | jtv: r=me. | 08:36 |
| jtv | adeuring: great, thanks | 08:37 |
| StevenK | adeuring: Would you like a review a small branch of mine too? | 09:02 |
| adeuring | StevenK: If it's a _small_ one, sure ;) | 09:03 |
| StevenK | adeuring: Is 144 lines too bug? | 09:03 |
| StevenK | s/bug/big/ | 09:03 |
| adeuring | OK, I'll look at it | 09:03 |
| StevenK | adeuring: No news is good news? :-) | 09:22 |
| adeuring | StevenK: yes, r=me | 09:30 |
| StevenK | adeuring: Thanks! | 09:30 |
| jtv1 | gmb: congratulations Recipient! Fate has drawn your name. Today could be the day, dear Recipient, that you review jtv's branch. Collect your prize here: https://code.edge.launchpad.net/~jtv/launchpad/recife-check-conflicts/+merge/33494 | 09:46 |
| === jtv1 is now known as jtv | ||
| StevenK | Bwahaha | 09:47 |
| jtv | Don't laugh—if it works for the direct marketers, why shouldn't it work for me? | 09:47 |
| lifeless | because they try 10K for every bite | 09:48 |
| lifeless | and if you do that in this channel... it will end badly | 09:48 |
| === adeuring1 is now known as adeuring | ||
| gmb | jtv, Sorry, wasn't ignoring your ping; have had to deal with home things. I'll be starting my shift in ~45 minutes, so I'll look then. Don't worry if your day ends before then, though. | 10:18 |
| jtv | gmb: no worries—actually I wasn't even aware that today was your shift. :-) | 10:18 |
| jtv | Just trying someone at random. | 10:18 |
| jtv | Fate picked another winner, Recipient! | 10:18 |
| gmb | :) | 10:19 |
| * gmb goes to finish pre-shift stuff | 10:20 | |
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | gmb: if you had any questions pent up, I'm back | 11:30 |
| gmb | jtv, No questions; it's r=me :) | 11:30 |
| === 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 | ||
| jtv | gmb: Thank you, Recipient, for your interest in our product! | 11:31 |
| gmb | :) | 11:31 |
| jtv | Hmm… this tack seems to work. I should do it more often. | 11:31 |
| gmb | jtv, You need to be careful on whom you try it. See, me, I'm polite and friendly to all comers. But if you ask the wrong person they'll try to /kick you before you get to finish your pitch. | 11:32 |
| jtv | gmb: I'm not worried… people know me here | 11:33 |
| jtv | …hey, what happened? | 11:34 |
| gmb | Heh. | 11:34 |
| jtv | Okay, maybe I won't try that again then. :-) | 11:35 |
| jtv | gmb: if you're not busy, I've got another small one in the backlog here. Converts a test to use the FakeLibrarian (speeding it up by about 20%). https://code.edge.launchpad.net/~jtv/launchpad/use-fakelibrarian/+merge/33324 | 12:01 |
| gmb | jtv, Sure | 12:02 |
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | Thanks Rec^H^H^Hgmb! | 12:02 |
| gmb | jtv, r=me | 12:10 |
| === 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 | ||
| jtv | gmb: thanks once more | 12:10 |
| gmb | np | 12:11 |
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: vittles || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| jtv | gmb, can I chuck a modest one on the queue? | 13:33 |
| gmb | jtv, Sure. I'll take a look in about 20 minutes | 13:34 |
| jtv | gmb: splendid | 13:35 |
| === jtv changed the topic of #launchpad-reviews to: On call: gmb || reviewing: vittles || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| gmb | jtv, Is there any reason that the new pagetest, which isn't really a user story, just a set of tests in doctest form, can't be written as a unittest? | 13:56 |
| gmb | That would fit better with our policy of not adding stories and doctests where they aren't needed. | 13:56 |
| jtv | gmb: mostly just laziness on my part tbh… I think you're right. | 13:56 |
| * jtv twiddles | 13:57 | |
| jtv | gmb: this will take a bit of course | 13:57 |
| gmb | jtv, No problem. I'll happily re-review when it's done. | 13:58 |
| jtv | thanks! | 13:58 |
| === 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 | ||
| gmb | jtv, Other than that it's absolutely fine. | 13:58 |
| jtv | \o/ | 13:58 |
| jtv | Of course "other than that" is the remaining 2 lines or so of change. But still, nice to hear. :) | 13:59 |
| jtv | gmb: I have the test converted and, in the process, found intimately related "stories" tests that I could move over into the same unit tests. Pushing. | 14:16 |
| gmb | jtv, Cool. | 14:16 |
| * gmb grabs a drink before looking | 14:17 | |
| === Ursinha-afk is now known as Ursinha | ||
| gmb | jtv, I wouldn't object if you removed the VWS from those tests, but that's a preference thing. r=me either way. | 14:29 |
| gmb | Nice work :) | 14:29 |
| jtv | thanks! <beam> | 14:29 |
| jtv | I like blank lines to separate setup, execute, verify stages. But yes, in this case there's not much point. I'll try it on for size. | 14:30 |
| jtv | Yup, shorter works better in this case. | 14:31 |
| leonardr | gmb, i need a follow-up review of https://code.edge.launchpad.net/~leonardr/launchpad/optimized-length-2/+merge/33417 (the latest revision only) | 15:04 |
| leonardr | it's a little complicated so i'm happy to ask questions | 15:04 |
| gmb | leonardr, Okay, just OTP at the moment; will look shortly. | 15:05 |
| leonardr | np | 15:05 |
| gmb | leonardr, Looking nwo. | 15:25 |
| gmb | leonardr, So, is it just r11392 that I need to look at? | 15:26 |
| leonardr | gmb, right | 15:26 |
| gmb | Righto | 15:26 |
| gmb | leonardr, Nice XXX explaining the problem. | 15:29 |
| gmb | leonardr, No further explanations needed; r=me. | 15:29 |
| leonardr | cool | 15:30 |
| leonardr | we'll see if the tests pass | 15:30 |
| EdwinGrubbs | abentley: when you have time, can you review the changes I made to my branch? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-615655-unicode-oops/+merge/33428 | 15:38 |
| abentley | EdwinGrubbs, I am tempted to think that your binary header handling should still be performed in message_from_string, so that it is easier for others to reuse. | 15:49 |
| abentley | EdwinGrubbs, btw, 5.5% of the messages in my inbox use "8bit" or "binary" content-transfer-encoding. | 15:51 |
| abentley | EdwinGrubbs, also, the standard mandates CRLF, not LF or CR as the header separators. Do you know whether the existing parser treats CR and LF as equivalent to CRLF? | 15:53 |
| EdwinGrubbs | abentley: is there a different message_from_string() function? I only see the one on the email module. | 15:53 |
| abentley | EdwinGrubbs, I may be confused. There's one for creating a SignedMessage from a string, I believe. | 15:54 |
| EdwinGrubbs | abentley: yes, the email module treats all three different line endings as possibly splitting the header and body. | 15:56 |
| abentley | EdwinGrubbs, okay, I think it would be nice to provide this as a utility function in lp.services.mail, but I won't require it. r=me. | 15:59 |
| EdwinGrubbs | abentley: thanks | 15:59 |
| === Ursinha is now known as Ursinha-lunch | ||
| === matsubara is now known as matsubara-lunch | ||
| === benji is now known as benji-lunch | ||
| jml | simple little patch: https://code.edge.launchpad.net/~jml/launchpad/subject-and-attachment/+merge/33544 | 17:04 |
| jtv | gmb: one more tiiiiny branch? https://code.edge.launchpad.net/~jtv/launchpad/bug-615673/+merge/33547 | 17:09 |
| gmb | jml, Looking now | 17:11 |
| gmb | jtv, You're up next. | 17:11 |
| gmb | First and last customer for the day :) | 17:11 |
| jtv | cool! | 17:11 |
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: jml || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jml | gmb: thanks. | 17:12 |
| gmb | jml, Approved. | 17:17 |
| === gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| gmb | jtv, r=me. | 17:19 |
| * gmb is done | 17:19 | |
| === gmb 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 | ||
| jtv | gmb: thanks again. You have earned your rest. | 17:19 |
| === matsubara-lunch is now known as matsubara | ||
| === Ursinha-lunch is now known as Ursinha | ||
| === benji-lunch is now known as benji | ||
| jml | Another small one: https://code.edge.launchpad.net/~jml/launchpad/testtools-0.9.6/+merge/33556 | 18:09 |
| jml | Yet another small patch: https://code.edge.launchpad.net/~jml/launchpad/launchpad-tester/+merge/33564 | 18:55 |
| === Edwin is now known as Guest13428 | ||
| === Guest13428 is now known as EdwinGrubbs | ||
| === matsubara is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!