=== matsubara is now known as matsubara-afk [00:59] mwhudson: (i'm here now) [00:59] poolie: ah yes, probably better [01:00] poolie: nope looks great [01:00] you need me to land it? [01:00] poolie: can you add a commit message? [01:06] sure [01:07] do i need any special formats or boilerplate or is that added automatically? [01:07] 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] poolie: no, ec2 land adds the boilerplate [01:08] done [01:08] poolie: thanks, launching ec2 land now [01:10] thanks [01:10] i guess i should work out how to do that myself [01:11] * mwhudson afk === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [09:52] .eaw [09:52] Oops. [11:43] adeuring: got one for you! https://code.launchpad.net/~jtv/launchpad/bug-645925/+merge/36540 [11:43] jtv: sure, I'll look [11:44] thanks === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === danilo_ is now known as danilos [11:54] 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] 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] 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] But some kind of SELECT check before doing an INSERT, or an INSERT later becoming an INSERT/SELECT/UPDATE is quite natural. [11:58] erm, I think its not the tests that make it a guessing game, I think ;) [11:59] That's right, but the tests should also reflect what goes on. [11:59] Not "err it's probably one of these, I'll just test against all of them." [11:59] hm, yes... OK, so r=me [12:00] Thanks! [12:00] 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. === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [13:22] 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] noodles775, yes, sinzui is my mentor [13:26] I will do it shortly [13:26] Great, thanks guys. === sinzui changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: - || queue: [sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || Reviewing: sinzui || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || Reviewing: sinzui || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:12] good morning adeuring === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || Reviewing: sinzui, sinzui || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === matsubara-afk is now known as matsubara [14:25] Thanks sinzui - I'll update to the radio buttons now. [14:30] morning bac === adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || Reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:31] sinzui: r=me [14:31] thanks [14:34] hi salgado, can you do a UI review for me, por favor? [14:35] bac, claro! [14:36] bueno! https://code.edge.launchpad.net/~bac/launchpad/bug-643538-code/+merge/36377 [14:41] mrevell: do you have time today to do a text review? [14:42] Yes bac, certainly. I have a call at the top of the hour. I'll do it after that. [14:42] mrevell: ok, thanks [15:03] adeuring, bac: Mind if I push a branch into the fifo? [15:03] allenap: I'll look [15:03] allenap: not at all [15:03] adeuring: Thanks. [15:06] sinzui: what branch do you need to have reviewed? i don't see it on +activereviews [15:09] the one I asked jtv to review earlier this week === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || Reviewing: -, sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:13] sinzui: found it! MP-hide-n-seek makes it interesting [15:13] bac, how was it hidden? [15:14] abentley: he had assigned it to jtv, so it didn't show up under "Requested reviews I can do" [15:14] bac, adeuring: could you please review https://code.edge.launchpad.net/~abentley/launchpad/unauthorized-email/+merge/36504? [15:15] 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] abentley: I just started a review for Gavin... [15:16] abentley: or he could paste the URL when requesting a review. no biggie, i've located it. [15:16] abentley: please add to queue === adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || Reviewing: allenap, sinzui || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:29] 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] adeuring: Sure, and thanks :) === adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || Reviewing: abentley, sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:52] abentley: r=me === adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || Reviewing: -, sinzui || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [15:52] adeuring, thanks. [15:59] hi sinzui, i'm looking at test_product_change_owner_changes_entry_importer and am confused [16:06] bac: yes? [16:06] sinzui: it doesn't seem to test what it claims, as i read it [16:06] the final assert tests that the entry importer is still the old_owner [16:07] i expected it to be the new owner [16:09] i just got subscribed to the cdo mailing list...anyone know what that's about? === deryck is now known as deryck[lunch] [16:15] sorry, wrong channel for that question. === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || Reviewing: -, abentley || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:24] 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] the mp doesn't clearly say what was changed... [16:24] salgado: yes. did you see the screenshots? sorry for the unclear MP [16:24] no, I didn't [16:25] salgado: did i forget to put them? looks... [16:25] I was looking just at the description, which I got by email [16:25] * salgado checks the mp on the web UI [16:25] salgado, mrevell: please see the first comment on the MP [16:25] bac, they're there [16:26] sorry that if forgot to include it in the original text [16:26] it's fine; I should've looked at the web UI as well === Ursinha is now known as Ursinha-lunch [16:33] Hi Brad [16:34] bac, For the first screenshot, I have a couple of questions: [16:35] 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] mrevell: there is no reason. the text is basically the same as it used to be. improvements encouraged. [16:36] Thanks bac, I'll suggest some in the comment on the review, in that case. [16:37] I have no second question, it seems. At least not for screenshot 0. [16:37] ok [16:45] 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] sinzui: that makes sense === matsubara is now known as matsubara-lunch === benji is now known as benji-lunch [16:53] mars: ping [16:53] Hi bac [16:54] hi mars, i'm looking at your branch for the py2.5 checker. [16:54] ok [16:54] mars: what do you think of lifeless' suggestions? [16:54] to trash it? :) [16:54] mars: no [16:54] not to trash it but to add it to 'make check' [16:55] and to answer the question about lucid builders [16:56] It could be done [16:56] mars: you don't sound enthusiastic [16:57] 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] mars: yes, i agree that would cover it [16:57] 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] bac, so what to do about the MP status - rejected, I guess? [16:57] to be superceded at a later date [16:58] bac, I'll update it [16:58] mars: i'd mark it 'needs fixing' === adeuring changed the topic of #launchpad-reviews to: On call: bac || Reviewing: abentley || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [16:58] that is a good idea [16:58] i'll do it === deryck[lunch] is now known as deryck [16:59] mars: are you implying it'll be a while before you get to it? [16:59] bac, I don't know when I get to it again, so yes, I am implying that [16:59] could be Monday [16:59] could be... infinitely longer than Monday [17:00] 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] mars: there are some devs that regularly run make lint [17:01] bac, good point [17:02] mars: so i leave it up to you. land now with a big XXX or defer. [17:02] ok, I will do it that way, and add a comment about the follow-up [17:02] ok, great === salgado is now known as salgado-lunch === matsubara-lunch is now known as matsubara === Ursinha-lunch is now known as Ursinha === benji-lunch is now known as benji === bac changed the topic of #launchpad-reviews to: On call: bac || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [18:11] jcsackett: is your unknown-translations ready to land? can i mark it 'approved'? [18:12] there's still some debate going on with it, i believe. [18:12] oh, no, nevermind, sinzui marked it as cool too. [18:12] yes, go ahead. [18:13] bac^ [18:13] * jcsackett needs to keep better track of his email. === gary_poster is now known as gary-lunch [18:50] 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] so my question is: should I get it re-reviewed before landing, or just mark it "Approved" myself? [18:50] (https://code.edge.launchpad.net/~benji/launchpad/wadl-refactoring/+merge/36452 is the MP in question) [19:01] benji: generally "needs fixing" means it should be looked at again before landing [19:01] benji: the alternative is for a reviewer to suggest some (generally trivial) changes but mark it 'approved' the first go round. [19:02] bac: in that case: tag, you're it [19:02] benji: gladly === salgado-lunch is now known as salgado [19:28] hey benji [19:28] 284 self.assertIsInstance(matcher.match("foo"), DoesNotContain) [19:28] can you swap those so it is expected, actual [19:28] import _pythonpath # not lint, actually needed [19:29] sure [19:29] Capitalize and add a period to the comment so that it is a complete sentence === gary-lunch is now known as gary_poster [19:29] 20from lp.testing.matchers import ( [19:29] 21 StartsWith, [19:29] 22 Contains, [19:29] 23 ) [19:29] alphamotize [19:29] * bac hmm i guess i should add these to the MP [19:30] bac: actually, assertIsInstance won't work like that; it's defined as taking (obj, class) [19:30] benji: ok, nm then. :) [19:32] the other two changes are made and as soon as the tests finish will be pushed [19:34] benji: i snuck in a third... :) [19:34] but i'm done [19:34] k :) [19:43] 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] sinzui: well if you're unhappy with it, i'm unhappy with it. [19:43] I will clean up my work and explain my sour victory [19:43] ok === bryceh changed the topic of #launchpad-reviews to: On call: bac || Reviewing: - || queue: [bryceh] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:17] bac, got time for another in the queue? [20:18] https://code.edge.launchpad.net/~jcsackett/launchpad/private-team-to-private-team-519306/+merge/36589 [20:18] (it's quite small) === jcsackett changed the topic of #launchpad-reviews to: On call: bac || Reviewing: - || queue: [bryceh, jcsackett] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:22] jcsackett: sure [20:22] thanks, bac. [20:29] jcsackett: how about using httplib.BAD_REQUEST instead of a magic number, albeit a universally recognized magic number? === sinzui changed the topic of #launchpad-reviews to: On call: bac || Reviewing: - || queue: [bryceh, jcsackett, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:31] bac: I put myself back in the queue for a re-review [20:31] sinzui: ok [20:32] in the webservice_error(), bac? i'd be good with that. [20:32] jcsackett: yeah. well, s/400/.../ in your files [20:33] dig. i can certainly do that. [20:34] jcsackett: r=bac [20:35] with three little requests [20:35] all good ones, bac. i'll take care of it. thanks! [20:35] 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] jcsackett: do you have another or was it just the one? [20:36] that's it. === bac changed the topic of #launchpad-reviews to: On call: bac || Reviewing: bryceh || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [20:38] bac, sorry, you'd sounded busy === bac changed the topic of #launchpad-reviews to: On call: bac || Reviewing: sinzui || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [21:13] sinzui: originally you expected the notify to just happen automatically when the attribute changed and that's just not how it works? [21:14] bac: there is no notification. none, nada [21:14] sinzui: but that is because you/we misunderstood the mechanism or is something busted? [21:14] I misunderstood [21:15] ok. i encounter the notify mechanism so infrequently that i don't have a good grasp of it [21:15] 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] I should delete the pretend subscriber [21:16] 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] 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] 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] 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] sinzui: looking into it thanks [21:53] sinzui: okay, I can see the unsubscribe issue you've described but cancel works for me [21:53] 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 [21:54] no cancel is also broken on that page, I see the bug page instead of the +subscriptions page [21:55] and I pushed revision 11483 right? [22:01] 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] bdmurray, I think the issue is that I am seeing radio button, but the widget should be checkboxes. [22:05] 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] no, you added it to a base class, I see in the MP class PersonSubscriptionsView(BugTaskSearchListingView): [22:07] 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 === Ursinha is now known as Ursinha-afk === salgado is now known as salgado-afk === Ursinha-afk is now known as Ursinha