[00:59] <poolie> mwhudson: (i'm here now)
[00:59] <mwhudson> poolie: ah yes, probably better
[01:00] <mwhudson> poolie: nope looks great
[01:00] <mwhudson> you need me to land it?
[01:00] <mwhudson> poolie: can you add a commit message?
[01:06] <poolie> sure
[01:07] <poolie> do i need any special formats or boilerplate or is that added automatically?
[01:07] <mwhudson> poolie: i need to scoot out in about 5 minutes, so if you can do that quickly i can fire up ec2 land before i go
[01:07] <mwhudson> poolie: no, ec2 land adds the boilerplate
[01:08] <poolie> done
[01:08] <mwhudson> poolie: thanks, launching ec2 land now
[01:10] <poolie> thanks
[01:10] <poolie> i guess i should work out how to do that myself
[01:11]  * mwhudson afk
[09:52] <allenap> .eaw
[09:52] <allenap> Oops.
[11:43] <jtv> adeuring: got one for you!  https://code.launchpad.net/~jtv/launchpad/bug-645925/+merge/36540
[11:43] <adeuring> jtv: sure, I'll look
[11:44] <jtv> thanks
[11:54] <adeuring> jtv: you are changing the permissions for lp-main, brnachscanner and fiera, but you test only the permission for branchscanner. what about the other two users? Do we need these changes, are existing tests / real world code breaking without the extended permissions, or should we add tests?
[11:56] <jtv> adeuring: lp-main is just on general principle (for UI we want to add, manual db maintenance etc).  The fiera change was just for a bit of extra robustness in case the exact interaction between the ORM and the db changes.
[11:57] <jtv> I originally had a test that tested against several db users, but ended up culling the ones that weren't directly relevant because I felt it turned the whole db access model into too much of a guessing game.
[11:58] <jtv> But some kind of SELECT check before doing an INSERT, or an INSERT later becoming an INSERT/SELECT/UPDATE is quite natural.
[11:58] <adeuring> erm, I think its not the tests that make it a guessing game, I think ;)
[11:59] <jtv> That's right, but the tests should also reflect what goes on.
[11:59] <jtv> Not "err it's probably one of these, I'll just test against all of them."
[11:59] <adeuring> hm, yes... OK, so r=me
[12:00] <jtv> Thanks!
[12:00] <jtv> Believe me, I didn't _feel_ like limiting it to 1 db user. :)  I just felt that it was one giant copout until I did—not really better than just giving everyone permission or using the write group.
[13:22] <noodles775> salgado: hi! is someone meant to mentor your ui review from yesterday? If so, I'd be keen for them to do it before I switch to use the radio buttons etc.
[13:25] <salgado> noodles775, yes, sinzui is my mentor
[13:26] <sinzui> I will do it shortly
[13:26] <noodles775> Great, thanks guys.
[14:12] <bac> good morning adeuring
[14:25] <noodles775> Thanks sinzui - I'll update to the radio buttons now.
[14:30] <adeuring> morning bac
[14:31] <adeuring> sinzui: r=me
[14:31] <sinzui> thanks
[14:34] <bac> hi salgado, can you do a UI review for me, por favor?
[14:35] <salgado> bac, claro!
[14:36] <bac> bueno!  https://code.edge.launchpad.net/~bac/launchpad/bug-643538-code/+merge/36377
[14:41] <bac> mrevell: do you have time today to do a text review?
[14:42] <mrevell> Yes bac, certainly. I have a call at the top of the hour. I'll do it after that.
[14:42] <bac> mrevell: ok, thanks
[15:03] <allenap> adeuring, bac: Mind if I push a branch into the fifo?
[15:03] <adeuring> allenap: I'll look
[15:03] <bac> allenap: not at all
[15:03] <allenap> adeuring: Thanks.
[15:06] <bac> sinzui: what branch do you need to have reviewed?  i don't see it on +activereviews
[15:09] <sinzui> the one I asked jtv to review earlier this week
[15:13] <bac>  sinzui: found it!  MP-hide-n-seek makes it interesting
[15:13] <abentley> bac, how was it hidden?
[15:14] <bac> abentley: he had assigned it to jtv, so it didn't show up under "Requested reviews I can do"
[15:14] <abentley> bac, adeuring: could you please review https://code.edge.launchpad.net/~abentley/launchpad/unauthorized-email/+merge/36504?
[15:15] <abentley> bac, ah.  Stale data, then.  Dunno how that could be fixed.  Obviously he can reassign it to you or one of your teams.
[15:16] <adeuring> abentley: I just started a review for Gavin...
[15:16] <bac> abentley: or he could paste the URL when requesting a review.  no biggie, i've located it.
[15:16] <bac> abentley: please add to queue
[15:29] <adeuring> allenap: there is a somewhat terse XXX in 218 of the diff. could you exand it either into a "standards comopliant" XX or remove it? otherwise, a nice refactoring, r=me
[15:29] <allenap> adeuring: Sure, and thanks :)
[15:52] <adeuring> abentley: r=me
[15:52] <abentley> adeuring, thanks.
[15:59] <bac> hi sinzui, i'm looking at test_product_change_owner_changes_entry_importer and am confused
[16:06] <sinzui> bac: yes?
[16:06] <bac> sinzui: it doesn't seem to test what it claims, as i read it
[16:06] <bac> the final assert tests that the entry importer is still the old_owner
[16:07] <bac> i expected it to be the new owner
[16:09] <jcsackett> i just got subscribed to the cdo mailing list...anyone know what that's about?
[16:15] <jcsackett> sorry, wrong channel for that question.
[16:24] <salgado> bac, so, to see your changes I need to create a new project and, change the uses_code thing to all different states and check what the project's code page looks like?
[16:24] <salgado> the mp doesn't clearly say what was changed...
[16:24] <bac> salgado: yes.  did you see the screenshots?  sorry for the unclear MP
[16:24] <salgado> no, I didn't
[16:25] <bac> salgado: did i forget to put them?  looks...
[16:25] <salgado> I was looking just at the description, which I got by email
[16:25]  * salgado checks the mp on the web UI
[16:25] <bac> salgado, mrevell: please see the first comment on the MP
[16:25] <salgado> bac, they're there
[16:26] <bac> sorry that if forgot to include it in the original text
[16:26] <salgado> it's fine; I should've looked at the web UI as well
[16:33] <mrevell> Hi Brad
[16:34] <mrevell> bac, For the first screenshot, I have a couple of questions:
[16:35] <mrevell> 1. I feel the text looks a little tough to get into. Is there a technical reason, or would you otherwise object to, using a bullet list to present the options?
[16:36] <bac> mrevell: there is no reason.  the text is basically the same as it used to be.  improvements encouraged.
[16:36] <mrevell> Thanks bac, I'll suggest some in the comment on the review, in that case.
[16:37] <mrevell> I have no second question, it seems. At least not for screenshot 0.
[16:37] <bac> ok
[16:45] <sinzui> bac: I have no idea how I screwed up the test/code. The test is not running because the attr assignment is independent of the subscriber event. I want to show them connected so I need to refactor the whole testcase to ensure the all events are complete before I do the assertion
[16:46] <bac> sinzui: that makes sense
[16:53] <bac> mars: ping
[16:53] <mars> Hi bac
[16:54] <bac> hi mars, i'm looking at your branch for the py2.5 checker.
[16:54] <mars> ok
[16:54] <bac> mars: what do you think of lifeless' suggestions?
[16:54] <mars> to trash it? :)
[16:54] <bac> mars: no
[16:54] <bac> not to trash it but to add it to 'make check'
[16:55] <bac> and to answer the question about lucid builders
[16:56] <mars> It could be done
[16:56] <bac> mars: you don't sound enthusiastic
[16:57] <mars> I would a) make it a stand-alone script; call that script from lint; call that script from 'make check'; add Py2.5 to the lucid builders
[16:57] <bac> mars: yes, i agree that would cover it
[16:57] <sinzui> bac: I am screwed. setting a field attr does not implicitly call notify with the ObjectModifiedEvent. I either need to restore the getter-setter for owner or I discover a wrapper that does what I expect
[16:57] <mars> bac, so what to do about the MP status - rejected, I guess?
[16:57] <mars> to be superceded at a later date
[16:58] <mars> bac, I'll update it
[16:58] <bac> mars: i'd mark it 'needs fixing'
[16:58] <mars> that is a good idea
[16:58] <bac> i'll do it
[16:59] <bac> mars: are you implying it'll be a while before you get to it?
[16:59] <mars> bac, I don't know when I get to it again, so yes, I am implying that
[16:59] <mars> could be Monday
[16:59] <mars> could be... infinitely longer than Monday
[17:00] <bac> mars: i disagree with lifeless on the 'wouldn't bother' part.  i'd say land what you have now...and commit to the changes outlined above in the near future.
[17:01] <bac> mars: there are some devs that regularly run make lint
[17:01] <mars> bac, good point
[17:02] <bac> mars: so i leave it up to you.   land now with a big XXX or defer.
[17:02] <mars> ok, I will do it that way, and add a comment about the follow-up
[17:02] <bac> ok, great
[18:11] <bac> jcsackett: is your unknown-translations ready to land?  can i mark it 'approved'?
[18:12] <jcsackett> there's still some debate going on with it, i believe.
[18:12] <jcsackett> oh, no, nevermind, sinzui marked it as cool too.
[18:12] <jcsackett> yes, go ahead.
[18:13] <jcsackett> bac^
[18:13]  * jcsackett needs to keep better track of his email.
[18:50] <benji> bac: I'm not sure what the procedure is on this: I have a branch which allenap reviewed yesterday and included a few comments, one of which he said was required to fix before landing, but marked his comment "Needs fixing".
[18:50] <benji> so my question is: should I get it re-reviewed before landing, or just mark it "Approved" myself?
[18:50] <benji> (https://code.edge.launchpad.net/~benji/launchpad/wadl-refactoring/+merge/36452 is the MP in question)
[19:01] <bac> benji: generally "needs fixing" means it should be looked at again before landing
[19:01] <bac> benji: the alternative is for a reviewer to suggest some (generally trivial) changes but mark it 'approved' the first go round.
[19:02] <benji> bac: in that case: tag, you're it
[19:02] <bac> benji: gladly
[19:28] <bac> hey benji
[19:28] <bac> 284        self.assertIsInstance(matcher.match("foo"), DoesNotContain)
[19:28] <bac> can you swap those so it is expected, actual
[19:28] <bac> import _pythonpath # not lint, actually needed
[19:29] <benji> sure
[19:29] <bac> Capitalize and add a period to the comment so that it is a complete sentence
[19:29] <bac> 20from lp.testing.matchers import (
[19:29] <bac>  21    StartsWith,
[19:29] <bac>  22    Contains,
[19:29] <bac>  23    )
[19:29] <bac> alphamotize
[19:29]  * bac hmm i guess i should add these to the MP
[19:30] <benji> bac: actually, assertIsInstance won't work like that; it's defined as taking (obj, class)
[19:30] <bac> benji: ok, nm then.  :)
[19:32] <benji> the other two changes are made and as soon as the tests finish will be pushed
[19:34] <bac> benji: i snuck in a third...  :)
[19:34] <bac> but i'm done
[19:34] <benji> k :)
[19:43] <sinzui> bac: I have a fix for my branch. I am unhappy with it. I now have a good understanding why fields do not notify change events.
[19:43] <bac> sinzui: well if you're unhappy with it, i'm unhappy with it.
[19:43] <sinzui> I will clean up my work and explain my sour victory
[19:43] <bac> ok
[20:17] <jcsackett> bac, got time for another in the queue?
[20:18] <jcsackett> https://code.edge.launchpad.net/~jcsackett/launchpad/private-team-to-private-team-519306/+merge/36589
[20:18] <jcsackett> (it's quite small)
[20:22] <bac> jcsackett: sure
[20:22] <jcsackett> thanks, bac.
[20:29] <bac> jcsackett: how about using httplib.BAD_REQUEST instead of a magic number, albeit a universally recognized magic number?
[20:31] <sinzui> bac: I put myself back in the queue for a re-review
[20:31] <bac> sinzui: ok
[20:32] <jcsackett> in the webservice_error(), bac? i'd be good with that.
[20:32] <bac> jcsackett: yeah.  well, s/400/.../ in your files
[20:33] <jcsackett> dig. i can certainly do that.
[20:34] <bac> jcsackett: r=bac
[20:35] <bac> with three little requests
[20:35] <jcsackett> all good ones, bac. i'll take care of it. thanks!
[20:35] <bac> hey bryceh i didn't see you'd snuck a review into the queue.  pinging the reviewer will get you a faster response.
[20:36] <bac> jcsackett: do you have another or was it just the one?
[20:36] <jcsackett> that's it.
[20:38] <bryceh> bac, sorry, you'd sounded busy
[21:13] <bac> sinzui: originally you expected the notify to just happen automatically when the attribute changed and that's just not how it works?
[21:14] <sinzui> bac: there is no notification. none, nada
[21:14] <bac> sinzui: but that is because you/we misunderstood the mechanism or is something busted?
[21:14] <sinzui> I misunderstood
[21:15] <bac> ok.  i encounter the notify mechanism so infrequently that i don't have a good grasp of it
[21:15] <sinzui> I was mislead by the existing subscriber to product change events. I assumed that since there was one and I could see no explicit calls for it, that field did the notify for us
[21:16] <sinzui> I should delete the pretend subscriber
[21:16] <bac> i think the change you have now is very good.  i've noted two requests in the MP
[21:18]  * sinzui makes changes
[21:31] <bdmurray> sinzui: my +subscriptions has been updated and is ready for a rereview - https://code.edge.launchpad.net/~brian-murray/launchpad/limited-subscriptions-page/+merge/35177
[21:31]  * sinzui looks
[21:49] <sinzui> bdmurray, I just had a very bad experience as sample person. I looked as my +subscriptions and decided to remove the blackhole one.
[21:49] <sinzui> bdmurray, https://bugs.launchpad.dev/ubuntu/hoary/+bug/2/+subscribe does not let me unsubscribe, and cancel takes me to a page I have not seen :(
[21:50] <bdmurray> sinzui: looking into it thanks
[21:53] <bdmurray> sinzui: okay, I can see the unsubscribe issue you've described but cancel works for me
[21:53] <sinzui> I think you watned to use checkboxes, https://bugs.launchpad.dev/jokosher/+bug/14/+subscribe is also wrong
[21:53] <_mup_> Bug #14: There is no link to the bugtrackers config page <Launchpad Bugs:Fix Released> <https://launchpad.net/bugs/14>
[21:54] <sinzui> no cancel is also broken on that page, I see the bug page instead of the +subscriptions page
[21:55] <bdmurray> and I pushed revision 11483 right?
[22:01] <bdmurray> sinzui: so bug 2 shouldn't have shown up in the result set as sample person is not a direct subscriber of that bug.  I've fixed the query in the view.
[22:03] <sinzui> bdmurray, I think the issue is that I am seeing radio button, but the widget should be checkboxes.
[22:05] <bdmurray> sinzui: I'm not sure how this is related to the work I've done.  I've only added ReturnToReferrerMixin to the +subscribe page
[22:06] <sinzui> no, you added it to a base class, I see in the MP class PersonSubscriptionsView(BugTaskSearchListingView):
[22:07] <sinzui> you did no write a story, and that is why it is surprising that this is broken. Let me attempt to write a real story and we can set the bar for minimum functionality