| === matsubara is now known as matsubara-afk | ||
| poolie | mwhudson: (i'm here now) | 00:59 |
|---|---|---|
| mwhudson | poolie: ah yes, probably better | 00:59 |
| 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:00 |
| poolie | sure | 01:06 |
| 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:07 |
| poolie | done | 01:08 |
| mwhudson | poolie: thanks, launching ec2 land now | 01:08 |
| poolie | thanks | 01:10 |
| poolie | i guess i should work out how to do that myself | 01:10 |
| * mwhudson afk | 01:11 | |
| === 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 | ||
| allenap | .eaw | 09:52 |
| allenap | Oops. | 09:52 |
| 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:43 |
| jtv | thanks | 11:44 |
| === 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 | ||
| 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:54 |
| 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:56 |
| 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:57 |
| 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:58 |
| 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 | 11:59 |
| 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. | 12:00 |
| === 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 | ||
| 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:22 |
| salgado | noodles775, yes, sinzui is my mentor | 13:25 |
| sinzui | I will do it shortly | 13:26 |
| noodles775 | Great, thanks guys. | 13:26 |
| === 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 | ||
| bac | good morning adeuring | 14:12 |
| === 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 | ||
| noodles775 | Thanks sinzui - I'll update to the radio buttons now. | 14:25 |
| adeuring | morning bac | 14:30 |
| === 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 | ||
| adeuring | sinzui: r=me | 14:31 |
| sinzui | thanks | 14:31 |
| bac | hi salgado, can you do a UI review for me, por favor? | 14:34 |
| salgado | bac, claro! | 14:35 |
| bac | bueno! https://code.edge.launchpad.net/~bac/launchpad/bug-643538-code/+merge/36377 | 14:36 |
| bac | mrevell: do you have time today to do a text review? | 14:41 |
| 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 | 14:42 |
| 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:03 |
| bac | sinzui: what branch do you need to have reviewed? i don't see it on +activereviews | 15:06 |
| sinzui | the one I asked jtv to review earlier this week | 15:09 |
| === 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 | ||
| bac | sinzui: found it! MP-hide-n-seek makes it interesting | 15:13 |
| abentley | bac, how was it hidden? | 15:13 |
| 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:14 |
| 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:15 |
| 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:16 |
| === 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 | ||
| 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:29 |
| === 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 | ||
| adeuring | abentley: r=me | 15:52 |
| === 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 | ||
| abentley | adeuring, thanks. | 15:52 |
| bac | hi sinzui, i'm looking at test_product_change_owner_changes_entry_importer and am confused | 15:59 |
| 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:06 |
| bac | i expected it to be the new owner | 16:07 |
| jcsackett | i just got subscribed to the cdo mailing list...anyone know what that's about? | 16:09 |
| === deryck is now known as deryck[lunch] | ||
| jcsackett | sorry, wrong channel for that question. | 16:15 |
| === 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 | ||
| 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:24 |
| 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:25 |
| 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:26 |
| === Ursinha is now known as Ursinha-lunch | ||
| mrevell | Hi Brad | 16:33 |
| mrevell | bac, For the first screenshot, I have a couple of questions: | 16:34 |
| 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:35 |
| 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:36 |
| mrevell | I have no second question, it seems. At least not for screenshot 0. | 16:37 |
| bac | ok | 16:37 |
| 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:45 |
| bac | sinzui: that makes sense | 16:46 |
| === matsubara is now known as matsubara-lunch | ||
| === benji is now known as benji-lunch | ||
| bac | mars: ping | 16:53 |
| mars | Hi bac | 16:53 |
| 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:54 |
| bac | and to answer the question about lucid builders | 16:55 |
| mars | It could be done | 16:56 |
| bac | mars: you don't sound enthusiastic | 16:56 |
| 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:57 |
| mars | bac, I'll update it | 16:58 |
| bac | mars: i'd mark it 'needs fixing' | 16:58 |
| === 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 | ||
| mars | that is a good idea | 16:58 |
| bac | i'll do it | 16:58 |
| === deryck[lunch] is now known as deryck | ||
| 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 | 16:59 |
| 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:00 |
| bac | mars: there are some devs that regularly run make lint | 17:01 |
| mars | bac, good point | 17:01 |
| 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 | 17:02 |
| === 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 | ||
| bac | jcsackett: is your unknown-translations ready to land? can i mark it 'approved'? | 18:11 |
| 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:12 |
| jcsackett | bac^ | 18:13 |
| * jcsackett needs to keep better track of his email. | 18:13 | |
| === gary_poster is now known as gary-lunch | ||
| 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) | 18:50 |
| 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:01 |
| benji | bac: in that case: tag, you're it | 19:02 |
| bac | benji: gladly | 19:02 |
| === salgado-lunch is now known as salgado | ||
| 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:28 |
| benji | sure | 19:29 |
| bac | Capitalize and add a period to the comment so that it is a complete sentence | 19:29 |
| === gary-lunch is now known as gary_poster | ||
| 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:29 | |
| benji | bac: actually, assertIsInstance won't work like that; it's defined as taking (obj, class) | 19:30 |
| bac | benji: ok, nm then. :) | 19:30 |
| benji | the other two changes are made and as soon as the tests finish will be pushed | 19:32 |
| bac | benji: i snuck in a third... :) | 19:34 |
| bac | but i'm done | 19:34 |
| benji | k :) | 19:34 |
| 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 | 19:43 |
| === 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 | ||
| jcsackett | bac, got time for another in the queue? | 20:17 |
| jcsackett | https://code.edge.launchpad.net/~jcsackett/launchpad/private-team-to-private-team-519306/+merge/36589 | 20:18 |
| jcsackett | (it's quite small) | 20:18 |
| === 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 | ||
| bac | jcsackett: sure | 20:22 |
| jcsackett | thanks, bac. | 20:22 |
| bac | jcsackett: how about using httplib.BAD_REQUEST instead of a magic number, albeit a universally recognized magic number? | 20:29 |
| === 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 | ||
| sinzui | bac: I put myself back in the queue for a re-review | 20:31 |
| bac | sinzui: ok | 20:31 |
| jcsackett | in the webservice_error(), bac? i'd be good with that. | 20:32 |
| bac | jcsackett: yeah. well, s/400/.../ in your files | 20:32 |
| jcsackett | dig. i can certainly do that. | 20:33 |
| bac | jcsackett: r=bac | 20:34 |
| 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:35 |
| bac | jcsackett: do you have another or was it just the one? | 20:36 |
| jcsackett | that's it. | 20:36 |
| === 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 | ||
| bryceh | bac, sorry, you'd sounded busy | 20:38 |
| === 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 | ||
| bac | sinzui: originally you expected the notify to just happen automatically when the attribute changed and that's just not how it works? | 21:13 |
| 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:14 |
| 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:15 |
| 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:16 |
| * sinzui makes changes | 21:18 | |
| 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:31 | |
| 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:49 |
| bdmurray | sinzui: looking into it thanks | 21:50 |
| 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:53 |
| sinzui | no cancel is also broken on that page, I see the bug page instead of the +subscriptions page | 21:54 |
| bdmurray | and I pushed revision 11483 right? | 21:55 |
| 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:01 |
| sinzui | bdmurray, I think the issue is that I am seeing radio button, but the widget should be checkboxes. | 22:03 |
| 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:05 |
| sinzui | no, you added it to a base class, I see in the MP class PersonSubscriptionsView(BugTaskSearchListingView): | 22:06 |
| 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 | 22:07 |
| === Ursinha is now known as Ursinha-afk | ||
| === salgado is now known as salgado-afk | ||
| === Ursinha-afk is now known as Ursinha | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!