| === jcsackett_ is now known as jcsackett | ||
| === Ursinha is now known as Ursinha-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
| === henninge changed the topic of #launchpad-reviews to: call: henninge || reviewing: - || queue: [https://code.edge.launchpad.net/~lifeless/launchpad/milestones/+merge/32855] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === jelmer is now known as Guest73756 | ||
| === Guest73756 is now known as jelmer | ||
| === jelmer is now known as Guest76162 | ||
| === Guest76162 is now known as jelmer | ||
| === henninge_ is now known as henninge | ||
| === henninge changed the topic of #launchpad-reviews to: call: henninge || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | henninge: I have a couple of branches to review. Do you fancy taking either of them on? | 10:48 |
|---|---|---|
| henninge | allenap: sure, which one do you want done first? | 10:48 |
| allenap | henninge: I don't have a preference. refactor-mailnotification is a bit long, but will be a bit boring and simple. cache-experiment might be more interesting for you. | 10:51 |
| henninge | allenap: ok, I'll have a look and take my pick ;-) | 10:51 |
| === StevenK changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | henninge: Thanks :) | 11:10 |
| henninge | allenap: lifeless was working on cachedproperty, too. Is this related somehow? | 11:11 |
| allenap | henninge: Kind of. I saw the cachedproperty code getting a bit ugly, and I'd long had a vague idea for how to go about it differently, so I tried it out. It seems good, to my eyes at least :) | 11:12 |
| henninge | allenap: yes, I understand what you mean, I felt similar about 'ugly'. ;-) Did you have a pre-imp discussion with somebody, though? | 11:15 |
| henninge | and I notice that the branch is called an "experiement" ... | 11:15 |
| allenap | henninge: No. It was just an experiment at first, but it went better than I expected so I cleaned it up for review. | 11:15 |
| henninge | allenap: I see you using tests in docstrings. I seem to remember that we have been moving these into doctest files and not introducing new ones. | 11:17 |
| allenap | henninge: I'm happy to move them. | 11:18 |
| allenap | henninge: Would you like me to move them before you continue? | 11:18 |
| henninge | allenap: I am not sure if that is an actual rule that we follow but I have not seen anybody using them before (except for lifeless). | 11:19 |
| henninge | allenap: so, yes, I'd prefer them being moved. | 11:19 |
| allenap | henninge: Okay, I'll do that now. | 11:19 |
| henninge | allenap: I like the new interface, btw... ;-) | 11:30 |
| allenap | henninge: Brilliant :) | 11:30 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: allenap || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| allenap | henninge: Done; the documentation is now in doc/propertycache.txt. | 11:34 |
| henninge | cool ;) | 11:34 |
| henninge | allenap: aren't these two calls identical? Do you really still need both? | 11:39 |
| henninge | 120 clear_cachedproperties(self) | 11:39 |
| henninge | 121+ IPropertyCacheManager(self).clear() | 11:39 |
| allenap | henninge: They're not identical yet; they clear different caches. I will remove clear_cachedproperties in a subsequent branch. | 11:39 |
| henninge | allenap: so, both implementation of cachedproperty exist in parallel atm and it depends on which one you import? | 11:41 |
| allenap | henninge: Yes. This branch was getting too long, and I wanted to see if it would pass review before doing more work. They won't interfere with one another; it's safe. | 11:42 |
| henninge | allenap: could you please add a comment to those lines up there explaining why both are needed? Also, it would be nice if lifeless' comment was a proper XXX. | 11:43 |
| allenap | henninge: Okay. | 11:43 |
| henninge | allenap: the change in lib/canonical/configure.zcml is an unrelated drive-by fix, right? | 11:45 |
| allenap | henninge: Yes. I realised it made sense to put all the lp.services zcml in the same place while I was there. It's related to the branch, but it is a clean-up. | 11:46 |
| henninge | allenap: cool, I see other clean-ups, too. Thanks! ;-) | 11:47 |
| henninge | allenap: btw, are you sure this is supposed to go into lp.services? It is not really LP specific, is it? | 11:48 |
| henninge | I mean, is the whole "canonical" tree expected to go away eventually, or just "canoincal.launchpad"? | 11:49 |
| allenap | henninge: I don't know actually. | 11:50 |
| wgrant | I think canonical.* is meant to go into lp.* and lazr.* | 11:52 |
| allenap | henninge: Can we assume this is okay for now? | 11:54 |
| allenap | henninge: XXX and comment added. | 11:56 |
| henninge | allenap: oh yeah, I would not have asked you to move it, I was just wondering ... ;-) | 11:58 |
| allenap | henninge: Cool :) | 11:58 |
| allenap | henninge: I have to go and help with the kid's lunch now. Do you mind if I step out for ~1h? | 11:58 |
| henninge | allenap: Fits perfectly, I'll be out to lunch soon, too. ;-) | 11:59 |
| allenap | henninge: Cool, thanks. | 11:59 |
| bigjools | henninge: hi, I've got a small branch just pushed up, can I add it to your queue please? | 12:07 |
| StevenK | bigjools: Get in line! | 12:07 |
| StevenK | :-P | 12:07 |
| bigjools | StevenK: yes that's what adding to the queue generally means ... | 12:07 |
| henninge | bigjools: go ahead ;-) | 12:10 |
| === bigjools changed the topic of #launchpad-reviews to: On call: henninge || reviewing: allenap || queue: [StevenK, bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| bigjools | thanks henninge | 12:11 |
| bigjools | it was quite tempting to jump StevenK for a laugh then :) | 12:11 |
| StevenK | Hmph :-P | 12:12 |
| henninge | StevenK: I assume that there is no obvious way to stormify the INSERT query in your branch? | 12:15 |
| * henninge has no idea, just checking ;) | 12:15 | |
| StevenK | henninge: I don't know how to Storm-ify stuff, and my last try ended in failure -- what's been added there for archivepermission is copied from other sections of the code | 12:18 |
| henninge | StevenK: I know it's being copied. ;-) I can imagine that it is not possible to Storm-ify that ... | 12:20 |
| henninge | so nm. | 12:20 |
| henninge | StevenK: review sent. r=me after some fixes ;) | 12:40 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: allenap || queue: [bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| StevenK | henninge: Thanks! | 12:40 |
| * henninge goes to lunch | 12:40 | |
| StevenK | henninge: topic =~ s/allenap/lunch/ ? | 12:41 |
| henninge | StevenK: no, I am not done with allenap's yet | 13:51 |
| StevenK | henninge: And no, it isn't the same call -- one is *sets*Included, the other is *sources*Included | 13:56 |
| henninge | StevenK: ah, now that you mention it ... ;-) | 13:57 |
| StevenK | henninge: And yes, relatedSets returns an ordered list | 13:59 |
| henninge | cool | 14:00 |
| StevenK | Doh! Alphabetical fail | 14:06 |
| henninge | Talking to me? ;) I saw that they were alphabetical but did not know if that was just coincidence because you added the entries in that order ... | 14:31 |
| StevenK | henninge: Would you like to mark the Status of my MP as Approved, or shall I do it? | 14:48 |
| henninge | allenap: Can you please extend DefaultPropertyCache's docstring to point out that getattr and setattr are already implemented by object? | 14:48 |
| allenap | henninge: Sure. | 14:48 |
| henninge | StevenK: You can do that yourself if you feel that you considered my comments enough ... ;-P | 14:49 |
| henninge | StevenK: I never set that when I review. It's for you to decide when your branch has all the reviews it needs. | 14:49 |
| henninge | allenap: or a comment | 14:50 |
| allenap | henninge: Oh yeah, a comment would make more sense actually. | 14:50 |
| henninge | allenap: I am done reviewing and I see what I like. | 14:55 |
| henninge | ;-) | 14:55 |
| allenap | henninge: \o/ Thanks | 14:55 |
| henninge | allenap: since this is a parallel implementation, I don't see a problem with landing it but I ask you to present this on the dev mailing list to see if people see problems with it. | 14:56 |
| allenap | henninge: Yeah, good idea. | 14:57 |
| henninge | Kind of a after-imp discussion... | 14:57 |
| henninge | Cool, r=me ;-) | 14:57 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: bigjools || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | bigjools: your turn ;) | 15:04 |
| bigjools | cool | 15:04 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: bigjools || queue: [allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| henninge | bigjools: Why did you create Test_rescueBuilderIfLost? Could this not have been part of TestBuilder? | 15:15 |
| bigjools | henninge: arse - it's because I forgot to remove the layer - it doesn't need any layers | 15:15 |
| bigjools | and it's also testing a standalone function | 15:16 |
| henninge | bigjools: builder.rescueIfLost? That is a method, not a function. Am I missing something? | 15:21 |
| bigjools | henninge: there's builder.rescueIfLost and rescueBuilderIfLost | 15:21 |
| bigjools | the former calls the latter | 15:21 |
| bigjools | it's a separate function for ease of testing, since it needs mock builders in some tests | 15:22 |
| henninge | bigjools: ok, I see. | 15:23 |
| henninge | bigjools: r=me ;-) | 15:24 |
| bigjools | henninge: actually I think I lied about the layer | 15:24 |
| bigjools | but | 15:24 |
| bigjools | I remember now I created the extra Test case so that we could add more tests for that function | 15:24 |
| bigjools | it's woefully under-tested | 15:24 |
| bigjools | henninge: and thanks :) | 15:24 |
| henninge | bigjools: welcome ;) | 15:25 |
| === henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [allenap, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| noodles775 | Hi henninge, if you've time before you leave, the MP is: https://code.edge.launchpad.net/~michael.nelson/launchpad/remove-bug-217644-workarounds/+merge/33397 | 15:55 |
| noodles775 | Thanks! | 15:55 |
| henninge | noodles775: I am sorry, I will stop reviewing now because I have to take care of something else. | 15:57 |
| henninge | I hope abentley will become available soon ... | 15:57 |
| === henninge changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [allenap, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| noodles775 | henninge: no probs. | 15:58 |
| abentley | henninge, I am just back from vacation and catching up on emails, but I can sign on, I guess. | 15:58 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [allenap, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| abentley | henninge, what's the status of allenap? | 15:59 |
| henninge | abentley: that is a second branch that I have not looked at yet. | 16:00 |
| abentley | henninge, okay. | 16:00 |
| henninge | abentley: I meant this one: https://code.edge.launchpad.net/~allenap/launchpad/refactor-mailnotification/+merge/32923 | 16:01 |
| henninge | aka "the long one" | 16:01 |
| henninge | abentley: the cache-experiment is done. | 16:01 |
| abentley | henninge, cool. | 16:01 |
| allenap | henninge: If it's too long, please let others jump the queue; it can wait. Also, it's entirely code moves so might not take too long to review. | 16:02 |
| henninge | allenap: ;-) It's not much longer than the cache-experiment branch is. | 16:02 |
| allenap | henninge: Mmm, yes, that did get a bit big :-/ | 16:03 |
| abentley | allenap, it looks okay, except that the copyright on date on newbug.py is wrong. | 16:08 |
| allenap | abentley: Oops! I'll fix that. Thanks. | 16:08 |
| allenap | How did I manage to get 2012 in there? | 16:09 |
| abentley | topic On call: abentley || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | 16:11 |
| abentley | noodles775, r=me | 16:17 |
| noodles775 | Thanks Aaron. | 16:17 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-lunch | ||
| leonardr | EdwinGrubbs: a while back you reviewed https://code.launchpad.net/~leonardr/lazr.restful/integrate-optimized-len/+merge/33023 for me | 16:52 |
| leonardr | you were about to approve it when i asked you to hold off because i'd identified a problem | 16:52 |
| leonardr | it turned out the problem had nothing to do with my branch (it was a bug in storm) | 16:52 |
| EdwinGrubbs | leonardr: I remember | 16:52 |
| leonardr | can you take another look at that branch and approve it? | 16:52 |
| EdwinGrubbs | leonardr: sure | 16:52 |
| EdwinGrubbs | leonardr: r=me | 16:53 |
| leonardr | thanks | 16:54 |
| === Ursinha is now known as Ursinha-lunch | ||
| gary_poster | abentley: could you review a super small branch, please? It just upgrades our zc.buildout and friends to the new releases. https://code.edge.launchpad.net/~gary/launchpad/buildout1.5.0/+merge/33410 | 17:13 |
| abentley | gary_poster, sure. | 17:13 |
| gary_poster | Thank you | 17:13 |
| abentley | gary_poster, r=me | 17:15 |
| gary_poster | thanks again | 17:15 |
| === jcsackett changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [jcsackett] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jcsackett | abentley, ping. | 17:25 |
| jcsackett | just letting you know, mp is at https://code.edge.launchpad.net/~jcsackett/launchpad/620494-downloads-sort/+merge/33413. | 17:25 |
| leonardr | abentley, i have a very small mp at https://code.edge.launchpad.net/~leonardr/launchpad/optimized-length-2/+merge/33417 | 17:44 |
| === leonardr changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [jcsackett,leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara-lunch is now known as matsubara | ||
| === Ursinha-lunch is now known as Ursinha | ||
| === benji is now known as benji-lunch | ||
| === sinzui changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [jcsackett,leonardr,sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| sinzui | abentley, I added to the queue. If you do not have time to review my branch, i'll hunt for someone. | 18:35 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: jcsackett || queue: [leonardr,sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| abentley | jcsackett, when you say "Implementation details: As above", it sounds like "Implementation details: talked with Curtis". Is that what you mean? | 18:47 |
| jcsackett | abentley: no, sorry, as above references Proposed fix. | 18:47 |
| jcsackett | bad write up on my part there, sorry. | 18:48 |
| jcsackett | i've updated the Description. | 18:48 |
| abentley | jcsackett, Cool. | 18:48 |
| abentley | jcsackett, ProductReleased.datereleased is a date, not a datetime? | 18:49 |
| jcsackett | abentley: the form that is presented when you create one expects datetime, but it appeared only date is used. | 18:50 |
| jcsackett | so time always == 0:00:00. | 18:50 |
| jcsackett | it is my understanding that Edwin-Grubbs has a better fix rolling out to address the more core issues. this is a bandaid on the edge-case. | 18:51 |
| jcsackett | er, EdwinGrubbs, rather. | 18:51 |
| abentley | jcsackett, why not change it use the actual time? | 18:51 |
| jcsackett | it's my understanding time is not used in the db, and we wanted the bandaid out before the next round of changes from db are rolled out. | 18:51 |
| jcsackett | I *believe* that EdwinGrubbs has something dealing with the db issues. | 18:52 |
| jcsackett | but it won't be available for a bit. | 18:52 |
| abentley | jcsackett, r=me. | 18:53 |
| jcsackett | thanks, abentley. :-) | 18:53 |
| === abentley changed the topic of #launchpad-reviews to: abentley || reviewing: leonardr || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| abentley | jcsackett, I think you should add an XXX on the sort explaining that it's a band-aid. Note our XXX policy: https://dev.launchpad.net/PolicyandProcess/XXXPolicy | 18:56 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: abentley || reviewing: leonardr || queue: [sinzui, Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| EdwinGrubbs | abentley: I added this branch to the queue. It's easy. https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-615655-unicode-oops/+merge/33428 | 18:57 |
| jcsackett | abentley: that's an excellent call. i will do that. | 18:57 |
| === benji-lunch is now known as benji | ||
| abentley | leonardr, r=me. | 18:58 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: leonardr || queue: [sinzui, Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: Edwin || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| abentley | EdwinGrubbs, in what context is it illegal for an email to have non-ascii characters? | 19:02 |
| === abentley changed the topic of #launchpad-reviews to: abentley || reviewing: sinzui || queue: [Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === deryck is now known as deryck[lunch] | ||
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| EdwinGrubbs | abentley: whenever a different encoding is used, it should be escaped as quoted-printable hence the "=3E" characters seen often, or as binhex. | 20:33 |
| === deryck[lunch] is now known as deryck | ||
| abentley | EdwinGrubbs, "should", perhaps, but definitely not "must". Content-transfer-encoding may be "8-bit" or "binary" for MIME messages in 8-bit encodings. | 20:34 |
| EdwinGrubbs | abentley: ok, then I think it is just the headers that are expected to be ascii, which is where the problem is occuring. | 20:38 |
| abentley | EdwinGrubbs, yes, those are illegal. | 20:38 |
| EdwinGrubbs | abentley: I can change it to just escape the headers then. | 20:40 |
| abentley | EdwinGrubbs, but it appears you're applying the escaping to the entire message, which would actually break valid messages. | 20:40 |
| EdwinGrubbs | abentley: yes, I'll have to split the headers and the body. | 20:41 |
| === matsubara is now known as matsubara-afk | ||
| abentley | EdwinGrubbs, I recommend tweaking message_from_string if possible. | 20:47 |
| EdwinGrubbs | ok | 20:48 |
| === abentley changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| leonardr | abentley, i need a pretty simple review | 21:00 |
| leonardr | https://code.edge.launchpad.net/~leonardr/launchpadlib/find-length/+merge/33440 | 21:00 |
| abentley | leonardr, I am good at giving simple reviews :-) | 21:00 |
| abentley | leonardr, is this based on your previous branch? | 21:01 |
| leonardr | abentley: well, it's related work, but it doesn't depend on the branch you reviewed earlier | 21:03 |
| abentley | leonardr, r=me | 21:04 |
| james_w | abentley: hi, could you review https://code.edge.launchpad.net/~lamont/launchpad/lp-buildd-70/+merge/33455 which is a merge of lamont's changes back in to launchpad to keep them in sync? | 22:18 |
| abentley | james_w, sorry, off for the day. | 22:18 |
| === abentley 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 | ||
| james_w | someone else then? | 22:19 |
| === Guest39999 is now known as jelmer | ||
| jelmer | james_w: Yes | 22:23 |
| james_w | thanks | 22:23 |
| james_w | I think it's a merge of two approved branches, but I need a review in order to ec2 land it | 22:23 |
| james_w | and it's backwards anyway, as the code is already in lp-buildd 70, so it's hard to disagree with it :-) | 22:23 |
| jelmer | hehe | 22:24 |
| jelmer | james_w: r68 mentions reverting all of r68, but I don't see those changes in the diff | 22:29 |
| jelmer | (and I don't understand why) | 22:29 |
| james_w | jelmer: yeah, that is odd | 22:30 |
| jelmer | james_w: see #soyuz | 22:34 |
| jelmer | james_w: as lamont has just updated the branch, r=me | 22:48 |
| james_w | thanks | 22:48 |
| === salgado is now known as salgado-afk | ||
| EdwinGrubbs | abentley: you're probably already gone for the day, but I made the changes you suggested if you're still around. | 23:18 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!