| jcsackett | anyone able to take a look at https://code.launchpad.net/~jcsackett/launchpad/archivemirror-timeout-618400 | 00:00 |
|---|---|---|
| jcsackett | er, https://code.launchpad.net/~jcsackett/launchpad/archivemirror-timeout-618400/+merge/46550 | 00:00 |
| === _mup__ is now known as _mup_ | ||
| thumper | https://code.launchpad.net/~thumper/launchpad/webservice-person-adapter/+merge/46658 | 20:03 |
| thumper | leonardr: want to review that branch? | 20:08 |
| leonardr | thumper: sure. one thing comes to mind right now | 20:10 |
| leonardr | i found all the other places we use a custom IFieldHTMLRenderer | 20:11 |
| leonardr | there's one we can replace with the generic person one | 20:11 |
| thumper | ok | 20:11 |
| thumper | we should do that in this branch ? | 20:11 |
| leonardr | maybe, or we could do another branch. the thing i want to bring up is it looks like there's one we can use for IText in general | 20:12 |
| leonardr | it obfuscates email addresses and calls text_to_html() | 20:12 |
| leonardr | it's used for bug description, mp commit message, and mp description | 20:12 |
| thumper | ok... | 20:13 |
| leonardr | even if we don't use it for itext in general, we should keep it in some generic place. right now there are two implementations. where should the single implementation go, and does that change our story for where we put the user formatter? | 20:13 |
| leonardr | while you chew on that i will look at the code | 20:13 |
| thumper | ok | 20:13 |
| thumper | I'm also in the taleo training meeting | 20:14 |
| leonardr | yeah | 20:14 |
| leonardr | sourcepackagerecipe.py seems to just have lint | 20:15 |
| thumper | leonardr: yes, there was some lint cleanup there | 20:15 |
| leonardr | aargh, my apidoc branch is being silently dropped on the floor. that's why it didn't land | 20:17 |
| leonardr | thumper: a comment before the assertion in test_person_renderer wold be nice, given that you start off with comments | 20:21 |
| leonardr | "render a recipe owner as a link" -> "render a person as a link to the person" | 20:23 |
| thumper | leonardr: ack | 20:24 |
| leonardr | the big comment on line 121 should be an xxx, and we should know more about it | 20:25 |
| leonardr | i'll try to find mars | 20:25 |
| thumper | leonardr: ok | 20:25 |
| leonardr | "a marker for people choices" -> "a marker for a choice among people" | 20:25 |
| thumper | leonardr: jml also did a review and has made similar comments :) | 20:26 |
| leonardr | everything else looks good | 20:26 |
| leonardr | ok, very good | 20:26 |
| leonardr | i cannot find mars, so i will try to figure out the <span> thing myself | 20:30 |
| thumper | leonardr: ok | 20:32 |
| thumper | leonardr: it is probably worthwhile doing a generic IText adapter | 20:33 |
| thumper | leonardr: I say JFDI (and I will) | 20:33 |
| leonardr | thumper: we have discovered a bug in the branch. the value sent into the renderer is not an object, it's a link to the object | 21:03 |
| thumper | ah... | 21:03 |
| thumper | what? | 21:03 |
| thumper | oh | 21:03 |
| thumper | the value bit | 21:03 |
| leonardr | thumper: basically, we need that field.__name__ thing | 21:04 |
| thumper | um... | 21:04 |
| thumper | so why does it work? | 21:04 |
| leonardr | it works in the unit test because we pass in the object | 21:04 |
| thumper | it worked interactively... didn't it? | 21:04 |
| * thumper checks | 21:04 | |
| leonardr | it worked interactively when you passed in the object | 21:04 |
| leonardr | value is the result of unmarshallField, which turns an object into a link | 21:05 |
| thumper | okay... | 21:05 |
| thumper | leonardr: yep... | 21:06 |
| * thumper fixes | 21:06 | |
| thumper | so... we'll need the bind then? | 21:06 |
| thumper | so what about IText? | 21:07 |
| thumper | hmm... perhaps consistency on the implementat bit | 21:08 |
| thumper | no, bind not needed | 21:13 |
| thumper | as we aren't getting the value from the field but the context | 21:13 |
| wgrant | jcsackett: https://code.launchpad.net/~wgrant/launchpad/bug-702819-bad-uris/+merge/46683 | 21:46 |
| === 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 | ||
| jtv | gmb: would you do me the honour of reviewing our bugzilla bugwatch branch? | 21:48 |
| jtv | This one: https://code.launchpad.net/~jtv/launchpad/bug-34102/+merge/46685 | 21:49 |
| gmb | jtv: Sure. I'll take a look presently. | 21:49 |
| jtv | Splendid. | 21:49 |
| thumper | EdwinGrubbs: https://code.launchpad.net/~thumper/launchpad/remove-webservice-xhtml-span/+merge/46686 | 21:52 |
| gmb | jtv: r=me with some nitpicks. | 22:02 |
| jtv | gmb: wow, that's fast. Pick away. :) | 22:02 |
| leonardr | i'd like a quick review of https://code.launchpad.net/~leonardr/launchpadlib/death-to-edge/+merge/46695 | 22:03 |
| leonardr | gary, maybe you'd be interested | 22:04 |
| leonardr | wgrant: take a look at the branch if you like, otherwise i'll land it in about 5 minutes | 22:26 |
| wgrant | leonardr: I am not yet a reviewer :( | 22:28 |
| leonardr | wgrant: you can still have an opinion. i'm going to self-review on lifeless's advice | 22:29 |
| wgrant | leonardr: It looks fine. | 22:33 |
| leonardr | ok | 22:33 |
| leonardr | awesome. the script i use to upload the release tarball uses edge, and now it's failing | 22:35 |
| lifeless | \o/ | 22:47 |
| === danilos changed the topic of #launchpad-reviews to: On call: - || reviewing: || queue: [jtv, danilo] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === jtv changed the topic of #launchpad-reviews to: On call: - || reviewing: || queue: [danilo] || 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!