[09:54] <james_w> https://code.edge.launchpad.net/~james-w/launchpad/fix-publishing-getbyid/+merge/28359 if someone has a minute please
=== james_w changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[09:54] <james_w> (assuming Edwin has gone to bed)
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: james_w || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[10:16] <jelmer_> 'morning James
=== StevenK changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: james_w || queue: [StevenK*2] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[10:35] <james_w> morning jelmer_
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [StevenK*2] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: StevenK || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[10:40] <jelmer_> looks like it's going to be a soyuz-heavy review day again :-)
[10:40]  * bigjools will also have 2 more later :)
[10:40] <StevenK> jelmer_: The two MPs are https://code.edge.launchpad.net/~stevenk/launchpad/expose-iarchive-newauth/+merge/28090 and https://code.edge.launchpad.net/~stevenk/launchpad/subscribers-can-view-p3as/+merge/27806
[10:48] <james_w> replied
[10:55] <jelmer_> james_w: r=me
[10:55] <james_w> thanks
[10:55] <james_w> writing tests for all the factory would be incredibly tedious
[10:56] <james_w> and generating them isn't much better than not having them.
[10:56] <james_w> but I agree that relying on indirect testing for it is not great
[10:56] <jelmer_> james_w: just checking that e.g. makeBinaryPackagePublishingHistory() returns something sensible ?
[10:57] <james_w> yeah, that would be ok
[10:57] <jelmer_> and takes into account the parameters its passed
[10:57] <james_w> but going through each parameter and specifying it, and then not specifying it and checking that you get something sensible created?
[10:58] <jelmer_> james_w: I guess there's a tradeoff decision to make there somewhere.
[10:58] <james_w> I guess you can leave some of that to Storm, because it won't let you set the wrong type to an attribute, or leave of a required one
[10:59] <james_w> I'm happy to start a testsuite and add the things that I just added if you like?
[10:59] <james_w> well, I would be if I wasn't working on a db patch, I've had enough of "make schema" for a little while
[10:59] <jelmer_> :-)
=== mrevell is now known as mrevell-lunch
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[11:42] <jelmer_> StevenK: r=me for the first branch, reviewing the second now
[11:49] <wgrant> jelmer_, StevenK: Shouldn't getPrivateSourcesList have its person argument forced to REQUEST_USER?
[11:49] <wgrant> letting anyone get anyone else's URL does not sound desirable.
[11:53] <wgrant> As for the other one, doesn't that let any subscriber use the API to poke around, negating the security benefits conveyed by the +packages restriction?
[11:54] <wgrant> +copy-packages, etc., too.
[12:15] <jelmer_> wgrant: good point (regarding REQUEST_USER)
[12:15] <jelmer_> wgrant: looking at the other issue now
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== mrevell-lunch is now known as mrevell
[12:38] <wgrant> jelmer_: What's your assessment of the permissions one?
=== kiko is now known as 40FAA4R6V
=== matsubara-afk is now known as matsubara
[13:04] <salgado> hi jelmer_.  care to review https://code.launchpad.net/~salgado/launchpad/remove-policy_options/+merge/28392 for me?
[13:05] <jelmer_> wgrant: following up to steve now
[13:05] <wgrant> jelmer_: Thanks.
[13:05] <jelmer_> StevenK: I was about to head out for lunch, but I'll have a look after.
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[13:08] <jelmer_> actually, I'll have a look now as it's a small branch
[13:08] <StevenK> wgrant: How would you recommend I change it to use REQUEST_USER always?
[13:09] <jelmer_> StevenK: use @call_with(user=REQUEST_USER)
[13:09] <wgrant> StevenK: @call_with(person=REQUEST_USER)
[13:09] <wgrant> And drop the @operation_parameters
[13:11] <StevenK> jelmer_, wgrant: http://paste.ubuntu.com/454419/
[13:11] <jelmer_> StevenK: yep
[13:13] <jelmer_> StevenK: You probably also have to update your web service API test, as it should refuse that argument now.
[13:14] <jelmer_> salgado: r=me, thanks for cleaning this up
[13:14] <StevenK> Right
[13:14] <salgado> jelmer_, cool, thanks!
[13:15]  * jelmer_ really goes to lunch this time
=== henninge_ is now known as henninge
[14:40] <rockstar> jelmer_, can I jump on your queue?
[14:51] <james_w> jelmer_: another soyuz one if you can fit it in :-) https://code.edge.launchpad.net/~james-w/launchpad/test-package-cloner/+merge/28405
=== bigjools changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [salgado, bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:56] <bigjools> 2 liner from me jelmer_
[15:06] <rockstar> jelmer_, my proposal is here: https://code.edge.launchpad.net/~rockstar/launchpad/recipe-security/+merge/28410
[15:18] <jelmer_> rockstar: sorry, was lunching - go ahead
[15:18] <rockstar> jelmer_, see my proposal above
[15:19]  * rockstar is one of those folks who likes to ASK to have a review, not just assume one...
[15:19] <rockstar> (like bigjools)
=== rockstar changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [salgado, bigjools, rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== bigjools changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [salgado, rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[15:20] <bigjools> pff
[15:20] <jelmer_> :-)
[15:22] <jml> rockstar, submitting a merge proposal _is_ asking for a review
[15:22] <rockstar> jml, I entirely disagree.
[15:22] <jml> rockstar, do go on.
[15:23] <rockstar> jml, it's the reviewees responsibility to get it reviewed, not the reviewers.
[15:23] <jelmer_> jml: my impression is that the stuff in the queue is what the current reviewer is going to process in their shift
[15:23] <rockstar> If the OCR decides to go out and look for reviews, good on him, but I don't think it's his job.
[15:23] <jelmer_> jml: Not sure if that's right but that's what I've observed so far :-)
[15:24] <rockstar> jml, basically, I believe in the "On Call Reviewer" philosophy, not "Review Lackey" philosophy.
[15:25] <jml> rockstar, nothing that you've said contradicts what I said.
[15:25] <jml> jelmer_, that does seem to be how it works.
[15:26] <rockstar> jml, indeed I think it does.  Throwing a ball for my dog doesn't necessarily imply "go fetch"
[15:27] <jml> rockstar, How is sending an email with text requesting a review and an attached patch to a list of developers with commit access *not* asking for a review?
[15:29] <jml> rockstar, it might not be asking the OCR for a review, it might be ineffective because contributors can't expect developers to have the time to incorporate their efforts, it might be counter to the process, but it is most certainly asking for a review
[15:29] <rockstar> jml, okay, so let's just settle on it being a terrible way to ask for a review, like standing in the middle of Paddington station and yelling "DOES ANYONE HAVE A PEN?"
[15:30] <rockstar> jml, if no one does the review, and it sits for a week, who's responsibility is it?
[15:30] <jml> rockstar, yours
[15:30] <rockstar> jml, yours == reviewee?
[15:31] <jml> rockstar, no, yours as a Launchpad developer with commit access. mine too, I guess, although I'm inactive.
[15:31] <rockstar> jml, yes, I could get to it, but I also have my own responsibilities and branches to land.
[15:32] <rockstar> jml, if you come to me and say "can I have a review?" I can say "Can you find someone else? I'm rather swamped" or "Sure"  Otherwise, I'm focusing on the work I need to do, and arranging my own reviews with others.
[15:33] <rockstar> jml, so yes, it's my responsibility (and yours) but it's one in many different responsibilities, and often not the highest priority to me.
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: james_w || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[15:33] <rockstar> So I guess by going to a OCR or another specific reviewer, you're asking for a higher priority on their list of things to do.
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: rockstar || queue: [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[15:34] <rockstar> jml, so you could say "Creating a merge proposal is asking for a review, but if you want it to get on someone's TODO list, you better ask them"
[15:35] <jelmer_> is *everybody* doing soyuz branches today?
[15:35] <rockstar> jelmer_, the line between code and soyuz is blurring at a rapid rate...  :)
[15:36] <rockstar> jelmer_, although truthfully, I'm finding the Soyuz problems becoming more and more interesting.
[15:37] <bigjools> ch..ch..ch...changes
=== matsubara is now known as matsubara-lunch
[16:15] <rockstar> jelmer_, howgoesit?
[16:18] <jelmer_> rockstar: almost done reviewing
[16:18] <rockstar> jelmer_, ack, thanks for doing this.
[16:19] <rockstar> jelmer_, I can take james_w's review (since I'm OCR next) if you want to EOD after me.
[16:29] <jelmer_> rockstar: that's alright, I'll be here for another 2 or 3 hours
[16:29] <rockstar> jelmer_, okay.
[16:30] <jelmer_> rockstar: although I won't stop you, it would mean a quicker review for James
[16:31] <rockstar> jelmer_, yeah, I need to start my shift now.
=== rockstar changed the topic of #launchpad-reviews to: On call: jelmer, rockstar || reviewing: rockstar, james_w || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:47] <bac> rockstar: i just read your conversation with jml.  my unsolicited thoughts:
[16:47] <rockstar> bac, they are solicited now
[16:47] <rockstar> bac, I was planning on copying the conversation and posting it to the list.
[16:47] <bac> 1) as an OCR with no requested reviews, I look to +activereviews and try to beat down any backlog.  i assume other reviewers do the same.
[16:48] <bac> 2) submitting a MP *is* asking for a review but in the most ineffective way possible.  it is still the submitters responsibility to *hunt down* a reviewer and make sure it gets done.
[16:48] <bac> 3) i can't think of a third.
[16:48] <rockstar> bac, I go through +activereviews as well, to an extent.  If the person isn't around to ask questions to, I usually just leave it alone.
[16:49] <rockstar> bac, in a way though, I do agree with jml that it's also the responsibility of the people who have commit rights.
[16:49] <bac> rockstar: i can see your point but that really disadvantages our colleagues in underrepresented time zones
[16:50] <jml> there's an analogy with a user asking a question on #launchpad
[16:50] <bac> i prefer doing on-call reviews that can be done interactively but have no problem just posing my questions in the review if the person is not available
[16:50] <rockstar> bac, this is true, but every time I get involved with jtv as reviewer, I always drag it out because we're in different time zones.  I don't think I'm the most effective reviewer for jtv.
[16:51] <rockstar> jml made a good argument that it's the committers responsibility, because this is an open source project, and I'd expect something similar on any project I contribute to.
[16:51] <rockstar> Granted I feel the need to be a squeaky wheel as well though.
[16:52] <bac> rockstar: we've had different experiences then that have led to different conclusions.  i think jtv and i work well on both sides of the review fence and have considered a little delay for him just to be part of living in BKK
[16:53] <bac> the review team should be pro-active but i still maintain it is the branch owner who must ultimately be responsible for getting a review.  if that process is difficult then that is egg on our faces and something we should fix.
[16:53] <rockstar> bac, ack, I completely agree with you.
[16:54] <bac> jelmer_, rockstar: great, now that we're in agreement, who'd like to do a review for me?  https://code.edge.launchpad.net/~bac/launchpad/bug-595907/+merge/28423
[16:54] <rockstar> bac, pop it on the queue.
=== bac changed the topic of #launchpad-reviews to: On call: jelmer, rockstar || reviewing: rockstar, james_w || queue: [bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:55] <bac> thanks
[17:12] <bac> rockstar, jelmer_: i am truly a despicable person.  after requesting an on-call review, i must step out for a bit.  feel free to skip over me if i'm not here when my number is called.
[17:13] <rockstar> bac, don't sweat it.  I need to chat with sinzui on post-imp call that should have been a pre-imp call at some point this morning.
=== deryck is now known as deryck[lunch]
[18:04] <jml> who would like to review https://code.edge.launchpad.net/~jml/launchpad/apidoc-dir/+merge/28432
[18:04] <jml> it's wafer thin.
[18:06] <jml> As someone once said, "All that is necessary for the triumph of evil is for good men to do nothing."
=== matsubara-lunch is now known as matsubara
[18:13] <jelmer_> jml: me me me
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer, rockstar || reviewing: jml, james_w || queue: [bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[18:14] <jml> jelmer_, thanks.
[18:15] <jelmer_> jml: I would expect that branch to also add an entry to .bzrignore
[18:16] <jml> jelmer_, I would likewise except the same!
[18:16]  * jml fixes
[18:16] <jelmer_> :-)
[18:19] <bac> jelmer_, rockstar: i've returned from a very refreshing lunch
[18:20] <jml> jelmer_, other than that, am I ok to land?
[18:20] <jelmer_> jml: Yeah, r=me
[18:21] <james_w> could someone check that I have submitted https://code.edge.launchpad.net/~james-w/launchpad/archive-job-db/+merge/28437 correctly?
[18:23] <jelmer_> james_w: Yeah, looks ok to me - against db-devel and db review from stub
[18:23] <jelmer_> I don't know what else there is to consider.
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer, rockstar || reviewing: bac, james_w || queue: [bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== jelmer_ changed the topic of #launchpad-reviews to: On call: jelmer, rockstar || reviewing: bac, james_w || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[18:24] <jelmer_> Somebody should write a bot to update the topic..
[18:24] <james_w> thanks
[18:33] <rockstar> jelmer_, my review is still pending...  :(
[18:33] <rockstar> jelmer_, I've also thought about having a bot update the topic.
=== salgado is now known as salgado-lunch
=== deryck[lunch] is now known as deryck
=== salgado-lunch is now known as salgado
[20:38] <bac> jelmer_: are you still working on my review?  any questions?
[20:49] <jelmer_> bac: sorry, yeah - kept getting interrupted but almost finished now
=== salgado is now known as salgado-brb
[20:49] <bac> ok, thanks jelmer_
[21:16] <jelmer_> bac: I'm getting strange test failures in distribution-mirror.txt that seem unrelated to your changes
[21:17]  * bac looks
[21:17] <jelmer_> bac: I can confirm that the pop-up icons are displayed and your changes are fine.
=== salgado-brb is now known as salgado
[21:23] <jelmer_> bac: r=me, it seems specific to my desktop machine
[21:23] <jelmer_> bac: my apologies for taking so long.
[21:23] <bac> jelmer_: i'm re-running those tests now and will only land the branch via ec2
[21:23] <bac> jelmer_: no worries.  you've had a very long day by now, i'm sure
=== sinzui changed the topic of #launchpad-reviews to: On call: jelmer, rockstar || reviewing: bac, james_w || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== rockstar changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk