| === jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [henninge, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| StevenK | wallyworld_: Do you need me to look at those MPs for you? | 04:28 |
|---|---|---|
| wallyworld_ | StevenK: yes please | 04:50 |
| StevenK | wallyworld_: In _test_recipebuild_listing, why are you removing the security proxy from naked_recipebuild.distribution ? | 04:54 |
| StevenK | And if it's called _test_... doesn't that mean it isn't called at all? | 04:54 |
| wallyworld_ | StevenK: from memory, if i didn't do that i got some sort of access exception | 04:55 |
| wallyworld_ | StevenK: let me switch to that branch to have a look | 04:55 |
| === jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: henninge || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| wallyworld_ | StevenK: it's called _test because that method is internal and called by other real test methods which first do some setup and then invoke the _test method | 05:01 |
| wallyworld_ | StevenK: the real test methods are test_recipebuild_listing_anonymous and test_recipebuild_listing_with_user | 05:01 |
| StevenK | Ah, that's fine then | 05:02 |
| wallyworld_ | StevenK: cool. took me a second to remember what i had done :-) | 05:04 |
| StevenK | The only other thing I can think of is different indentation for function definitions, but I can't remember rightly | 05:07 |
| StevenK | wallyworld_: You have one Approve and one Needs Fixing, enjoy | 05:23 |
| wallyworld_ | StevenK: thanks! | 05:23 |
| wallyworld_ | StevenK: with the imports, i had then all split, one per line, but i'm almost certain i was told by someone else that if it fits on one line, leave it like that. but i prefer the multiline approach myself and hence agree with you | 05:25 |
| StevenK | Odd | 05:26 |
| wallyworld_ | StevenK: i'll follow up and see if i misunderstood what i was being told :-) | 05:26 |
| === jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: abentley || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | StevenK: want another review to practice on? We're both on call I suppose but I can't very well review my own. :) | 06:10 |
| jtv | Also, if StevenK reviews my branch, I'll need a mentor! | 06:13 |
| jtv | Anyone else reviewing? | 06:24 |
| StevenK | jtv: I can have a look | 06:25 |
| jtv | StevenK: thanks—there should be some things you can shoot at. | 06:26 |
| StevenK | jtv: Done | 06:32 |
| jtv | thanks! | 06:32 |
| === jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: - || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | StevenK: can you live with the compromise I propose in my MP? | 07:21 |
| jtv | (Just submitted) | 07:21 |
| StevenK | jtv: Sounds good, Make It So | 07:23 |
| jtv | Thanks. And I thought "make lint" used to warn about dead variables… | 07:23 |
| jtv | StevenK: the diff has updated. Enough to change your vote? | 07:30 |
| StevenK | jtv: Done | 07:32 |
| jtv | thanks again | 07:32 |
| StevenK | jtv: Care to mentor my review of https://code.launchpad.net/~gary/launchpad/bug683115/+merge/42322 ? | 07:33 |
| jtv | heh, I was just looking at that one | 07:33 |
| === jtv changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: - || queue: [jtv*] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jtv | StevenK: (I wonder if I could mentor your review of my branch :-) | 07:34 |
| StevenK | jtv: Care to review https://code.edge.launchpad.net/~stevenk/launchpad/db-spph-ancestry/+merge/41943 ? | 08:25 |
| jtv | StevenK: of course. I may be a bit slow though, as today is do-or-die day for a year-long refactoring project. | 08:29 |
| jtv | StevenK: you have my code vote | 09:03 |
| === jelmer changed the topic of #launchpad-reviews to: On call: jtv, jelmer || Reviewing: - || queue: [jtv*] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jelmer | jtv: are those branches still pending? | 09:59 |
| jtv | jelmer: there's only one that I can see, but yes it is! | 09:59 |
| === jelmer changed the topic of #launchpad-reviews to: On call: jtv, jelmer || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| jtv | We emptied out the queue. Might as well get out while we're on top. | 11:46 |
| === jtv changed the topic of #launchpad-reviews to: On call: jelmer || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| salgado | jelmer, can you review a trivial one for me? (https://code.launchpad.net/~salgado/launchpad/bug-683106/+merge/42351) | 12:37 |
| jelmer | salgado: There's a (trivial) conflict in that branch, can you fix that? | 12:46 |
| salgado | jelmer, doing so now. :) | 12:47 |
| jelmer | salgado: Are you sure there's no way to use launchpadlib anonymous access at the moment? | 12:47 |
| salgado | jelmer, there is | 12:48 |
| salgado | it works out-of-the-box for everything but collections | 12:49 |
| salgado | in the case of collections, lazr.restful does a hard-coded check for launchpad.View, so for collections of objects that you want to expose to anonymous users, one must define launchpad.View adapters for those objects | 12:50 |
| jelmer | salgado: Sorry, I was a bit vague. I mean the launchpadlib_for_anonymous convenience function | 12:50 |
| jelmer | I thought we already had one, and was wondering if you'd checked for that. | 12:51 |
| salgado | oh, there's one helper which uses WebserviceHTTPCaller | 12:52 |
| salgado | but that's only available for pagetests, it doesn't allow you to specify the version of the webservice you want to use and doesn't provide real launchpadlib-like objects (you have to deal with json directly) | 12:53 |
| jelmer | what about Launchpad.login_anonymously() ? | 12:54 |
| jelmer | hmm, that probably uses the local users standard directory for launchpad credentials | 12:55 |
| salgado | yes, it would add the new credential to the cache, I think | 12:57 |
| jelmer | It's a pity that code has to be duplicated. It'd be great if we could override the default directory launchpadlib uses more easily. | 12:57 |
| jelmer | salgado: Anyway, that's beyond the scope of this branch. r=me | 12:58 |
| leonardr | salgado: i can rationalize this if you file a bug for it, but i probably can't get to it anytime soon | 12:58 |
| salgado | jelmer, I kind of agree with you there, but it's also arguable that having this extra helper together with the others will make this functionality more discoverable, and it's not that much code we're duplicating anyway | 13:00 |
| jelmer | salgado: It seems reasonable to have specific helpers in the Launchpad tree (e.g. with a service_root that defaults to "http://api.launchpad.dev/") but atm launchpadlib_for() and launchpadlib_for_anonymous() duplicate code that's in launchpadlib. It'd be nice if they could just be trivial wrappers around the launchpadlib code. | 13:01 |
| salgado | indeed | 13:02 |
| henninge | jtv: I saved recife the roll-back diff (just recife) and the inverse diff 10008..10007 from db-stable to files and compared the diffs. | 13:39 |
| henninge | I get this: http://paste.ubuntu.com/538642/ | 13:39 |
| henninge | Any idea why those lines differ? | 13:39 |
| henninge | ah, I just realized that they are context | 13:43 |
| henninge | jtv: approved | 13:53 |
| === matsubara is now known as matsubara-lunch | ||
| === salgado is now known as salgado-lunch | ||
| bac | #startmeeting | 15:00 |
| bac | doh | 15:00 |
| === salgado-lunch is now known as salgado | ||
| === matsubara-lunch is now known as matsubara | ||
| === deryck is now known as deryck[lunch] | ||
| salgado | leonardr, I didn't understand your comment earlier and you left right after you made it so I didn't have a chance to say so. care to clarify? | 16:10 |
| leonardr | salgado: sorry, i had to restart | 16:11 |
| leonardr | as your reviewer said, the idea of a special anonymous version of launchpad_for is a little hacky. i was offering to clean that up for you later if oyu filed a bug | 16:12 |
| salgado | leonardr, oh, to refactor Launchpad.login_anonymously() so that we can use that and avoid the extra helper? | 16:14 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jelmer, Edwin || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| leonardr | salgado: yeah, or let you pass in the NOBODY user to launchpad_for or something | 16:16 |
| === benji changed the topic of #launchpad-reviews to: On call: jelmer, Edwin, benji || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jelmer | hi Benji, Edwin | 16:31 |
| EdwinGrubbs | jelmer: hi | 16:32 |
| benji | EdwinGrubbs: I'd like to do some more mentored reviews today if we have any takers. | 16:38 |
| EdwinGrubbs | benji: surprisingly, I don't see any merge proposals that we can take from https://code.launchpad.net/launchpad/+activereviews | 16:44 |
| benji | yep, it seems unusually quiet | 16:44 |
| === deryck[lunch] is now known as deryck | ||
| === benji is now known as benji-lunch | ||
| salgado | leonardr, bug 683748 | 17:05 |
| _mup_ | Bug #683748: Get rid of launchpadlib_for_anonymous <Launchpad Foundations:New> <https://launchpad.net/bugs/683748> | 17:05 |
| jcsackett | benji: i can solve your quiet issue. here's an MP i could use some eyes on: https://code.edge.launchpad.net/~jcsackett/launchpad/anonymous-api-access-emails-681815/+merge/42309 | 17:07 |
| === jcsackett changed the topic of #launchpad-reviews to: On call: jelmer, Edwin, benji || Reviewing: - || queue: [jcsackett] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| jcsackett | naturally i do this at lunch time for maximum inconvenience. :-P | 17:08 |
| leonardr | salgado: thanks | 17:21 |
| === benji-lunch is now known as benji | ||
| benji | jcsackett: I should be able to do a (mentored) review for you | 18:25 |
| benji | jcsackett: I assume this is what you want reviewed? https://code.launchpad.net/~jcsackett/launchpad/anonymous-api-access-emails-681815/+merge/42309 | 18:29 |
| jcsackett | benji: yup, posted the link earlier. | 18:30 |
| jcsackett | mentored is fine. i'm in the same boat, so i'm happy to give you mentoring fodder. :-) | 18:30 |
| benji | :) | 18:31 |
| === benji changed the topic of #launchpad-reviews to: On call: jelmer, Edwin, benji || Reviewing: jcsackett || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| benji | jcsackett: ok, you have my approval; hopefully EdwinGrubbs will be able to give his seal of approval too | 18:43 |
| jcsackett | benji: cool, thanks! | 18:43 |
| benji | (I only had a small and non-binding comment about the long "want" lines) | 18:43 |
| * EdwinGrubbs looks at it | 18:44 | |
| EdwinGrubbs | jcsackett: when I ran your test without the fix to checkUnauthenticated(), it still passed. Does that happen for you? | 18:57 |
| jcsackett | EdwinGrubbs: yeah, i just noticed that in running some tests while preparing for land. | 18:58 |
| jcsackett | EdwinGrubbs: it looks like the webservice caller isn't getting emails regardless of how you go about it. | 18:58 |
| jcsackett | EdwinGrubbs: i'm going to put the MP to WIP and will ping you when i have it sorted. | 18:59 |
| * jcsackett sighs. | 18:59 | |
| jcsackett | EdwinGrubbs: i've pushed up changes, and added a comment explaining something i've discovered. | 19:22 |
| jcsackett | it seems the security hole this patches doesn't exist in testruns--an interaction against qastaging that shows the hole doesn't show the hole against dev. | 19:22 |
| jcsackett | at least, that's how it appears. in any event, i still was able to improve what the test was meant to show, and set it up so that it would catch any 404 errors we were worried about this change introducing. | 19:23 |
| EdwinGrubbs | jcsackett: have you asked leonardr? | 19:24 |
| leonardr | jcsackett, what's the problem? | 19:24 |
| jcsackett | leonardr: i was working on a patch to deal with this bug: https://bugs.launchpad.net/launchpad-registry/+bug/681815 | 19:24 |
| jcsackett | i can't create circumstances to make this bug happen against dev in unittests, though the scripts attached to the bug do demonstrate it to exist on qastaging. | 19:25 |
| === salgado is now known as salgado-afk | ||
| === matsubara is now known as matsubara-afk | ||
| jcsackett | EdwinGrubbs: you can take me out of the reviewing thing. after speaking with leonardr and creating a unit test that uses launchpadlib, i still can't create a test case that demonstrates this problem as expected. i'm going to take another look at it tomorrow. | 22:15 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: jelmer, Edwin, benji || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews | ||
| EdwinGrubbs | jcsackett: ok, good luck | 22:15 |
| jcsackett | EdwinGrubbs: i'm hoping fresh eyes tomorrow will be sufficient luck. but thanks. :-) | 22:16 |
| james_w | hi, could someone take a look at https://code.edge.launchpad.net/~james-w/launchpad/export-specification-bug-links/+merge/42426 ? | 22:23 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!