[03:32] thumper: can you look at https://code.edge.launchpad.net/~mwhudson/launchpad/vostok-traverse-distro/+merge/31241 again? [03:32] ok [03:32] mwhudson: do you have a few minutes for me to bounce some stuff off you? [03:32] skype like? [03:33] thumper: ok [03:34] thumper: i'm online [03:35] mwhudson: I'll have to defer for now, other people calling [03:35] thumper: ok [03:36] thumper: the call or the review or both? [03:36] the call [03:36] k [05:56] thumper: NotFound is a zope thing [05:56] it's not in lp.app.errors [05:56] so i'm going to ignore that comment? [06:03] um... [06:05] thumper: NotFound != NotFoundError [06:05] ah [06:05] yeah [06:05] I just worked that out [06:05] for better or worse [06:06] ok [06:06] something inside traversal converts the latter into the former [06:06] cool [07:23] jtv: Why do you want to cache the incumbent message in the traits helper? [07:23] * henninge is back to business ;) [07:23] hi henninge [07:23] henninge: good! :) [07:24] Hi lifeless! ;) [07:24] jtv: I am starting to see what Danilo meant by "heavy weight". [07:25] henninge: those are the helpers, and as I said, those are a bit legacy in that they may not be needed any more. [07:25] That's not the traits; it's what I extracted the traits out of. [07:25] jtv: I understand but I would have expected that to go into the class that uses the traits. [07:25] henninge: can't do that in the class that uses the traits. [07:26] do what, the caching? [07:26] The class that uses the traits can be a model class—not good to cache this sort of thing there. [07:26] We can do it in the code. [07:26] ok [07:27] But as I said, this is just what's left after extracting the traits—the caching is the possible reason not to remove the helpers entirely once we have the traits. [07:27] do you guys know much about the API export stuff? [07:27] And I'm not saying we need the caching, just that it's an appropriate and convenient place for it if we do. [07:27] lifeless: not that much [07:27] I see [07:28] henninge: cleaning up the helpers is a possible next step, but the branch was big enough. :) [07:28] Okay, so other users of the traits are not expected create helper classes, too? [07:28] henninge: no, not at all [07:29] As I explained yesterday: one class does it all, and the existence of an older helper class that I extracted them out of is merely a distraction in all this. [07:29] Look at the actual TranslationSideTraits class: is that heavyweight? [07:30] jtv: no, except ... [07:30] ;) [07:30] getCurrentMessage [07:31] The implementation is a bit daft because of the transient mess with getImportedTranslationMessage and getCurrentTranslationMessage. I'd prefer to replace the "if" with separate methods. [07:32] But that's all under the hood. [07:34] jtv: which "if" ? [07:35] henninge: inside getCurrentMessage… that's not what you meant? [07:35] sorry, yes. [07:37] jtv: Although I was wondering if a specific method like that belongs into the traits at all ... [07:37] If we had all queries stormified and available from the interfaces, then no. But in the real world it's a primitive. [07:38] jtv: what do you bean by "available from the interfaces"? [07:39] The interfaces underneath wherever we need this. [07:39] Sorry for being pita about this but I want to make sure I understand why I am on your side with this ... ;-) [07:39] No worries. It's good. [07:40] What I mean is, sometimes we'll need to ask for the current message for a given potmsgset, template & language on a given side. [07:40] If we could just return a Storm object for the right condition, then getFlag(TranslationMessage) would probably do the trick. [07:41] Of course we could also do that and go straight to Storm to ask for the current message, but that seems a bit messy. [07:43] jtv: You lost me on the last sentence [07:43] And you literally mean "getFlag(TranslationMessage)"? [07:44] I mean, on the class. [07:44] I mean: we could probably do without getCurrentMessage if we were willing to do something like Store.of(TranslationMessage).find(traits.getFlag(TranslationMessage)).one() [07:44] Yes [07:44] jtv: is there another class that will have the flags? [07:45] Yes, there might be a ClassAlias in Storm. [07:45] I'd like to see Store.of(TranslationMessage).find(traits.current_flag).one() [07:45] Oh [07:45] Well what you just said is actually a very good way to formulate it. [07:46] as long as its not one per-row-per-page :) [07:46] *If* we only have the one class, of course [07:47] jtv: I'd provide both. [07:47] a "current_flag" attribute for most use cases [07:47] and a "getCurrentFlag(cls)" for the alias case. [07:48] lifeless: maybe you know this one... I just evaluated TranslationMessage.is_current_ubuntu and ubuntu_traits.getFlag(TranslationMessage). They both return the same PropertyColumn object. Yet a Storm query will work with the former and not with the latter. [07:48] oh, it does not? [07:48] so the former is a class attribute [07:48] Yes [07:48] is the latter perhaps returned a securityproxied object? [07:49] I think the __repr__ would have told me… in both cases it says: [07:49] [07:49] But there must be _something_ with proxies, because the error says: [07:49] CompileError: Don't know how to compile type zope.security._proxy._Proxy of [07:49] ahha yes [07:49] :) [07:50] print dir(thing), and thing.__class__ [07:50] Heh... if I removeSecurityProxy then the query does work. Madness reigns. [07:51] jtv: no, it does not [07:51] lifeless: the __class__ are the same for both [07:51] thats interesting [07:51] it's because you regeistered TranslationSideTraitsSet as a securedutility. [07:51] "does not" referred to madness reigning, btw. [07:51] henninge: okay, but then why don't I see the proxy? [07:52] Right, took me a while to figure that out :) [07:52] jtv: that's waht proxies do. They pretend to be the real thing. [07:52] Okay, but this is an incredibly effective pretense! [07:53] At least, I had a memory of the proxies showing up when I printed objects in "make harness" [07:53] But yes, dir() exposes the difference. [07:55] jtv: another thing I wonder about it "other_side" [07:56] henninge: would you prefer something like "other_side_traits"? [07:56] Or would you prefer it to be a TranslationSide that you'd use to look up the traits? [07:57] jtv: no, I don't wonder. ;) [07:57] anymore [07:57] oh ok :) [07:57] jtv: but "other_side_traits" seems preferable. [07:57] OK [07:57] My wondering was about it not being set but I saw that it happens in TranslationSideTraitsSet.__init__. [07:57] That's good. [08:19] henninge: I renamed other_side and also removed MessageSideHelper.getFlag (because that code moved to the traits in its entirety) [08:39] jtv: all the calls to "removeSecurityProxy" in your test - are they a result of abel's work? [08:39] henninge: it's not unthinkable that they contributed, but the main thing is as in the cover letter: using Zope in tests that didn't use it before. [08:40] Sorry: it's not unthinkable that *it* contributed. [08:40] 653 external_template = self.factory.makePOTemplate() [08:40] 654 - external_template.productseries.product.official_rosetta = True [08:40] 655 + product = external_template.productseries.product [08:40] 656 + removeSecurityProxy(product).official_rosetta = True [08:41] jtv: how is zope involved here that was no before? [08:42] And I wonder why "official_rosetta" is not settable in the first place ... [08:42] henninge: layer change [08:42] Oh, I did not know that affected how the factory creates objects. [08:43] AIUI it doesn't, directly, but it means no proxies. [08:44] Correction again: running Zopelessly means no proxies. === 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 === adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: james_w || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [09:49] jtv: why is "test_getFlag_and_setFlag" one test? [09:50] henninge: because it tests consistency between the two, more than either one of them specifically. Not very imaginative, sorry. [09:50] jtv: ok, I see it now. [09:50] (There are two separate tests in the concrete test classes that tie the internal consistency of the traits to the reality of the two flags) [09:51] * jtv tries DSL again… [10:00] Well that was disappointing. Still no DSL. Wonder if a bill got lost again. [10:13] https://code.edge.launchpad.net/~stub/launchpad/cronscripts/+merge/31934 [10:14] adeuring: ^^ === stub changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: james_w || queue: [stub] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [10:14] stub: sure; let me just finish a review for james_w === adeuring1 changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: stub || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === adeuring1 is now known as adeuring === lifeless_ is now known as lifeless [10:59] henninge: thanks! [10:59] jtv: welcome [11:00] you were gone to check DSL when I wanted to tell you. Is it working again now?? [11:00] henninge: nope [11:08] stub: r=me [11:21] ta [11:24] henninge: can't lend using edge… I'll try again. [12:23] james_w, I'll take a look at your no-more-sampledata-2 branch. [12:23] (next week, I'm going to do less coding stuff, promise) === bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: stub || queue: [] || 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: stub, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [12:40] morning adeuring [12:40] hi bac! [12:41] adeuring: many takers today? [12:41] bac: a few. Could be worse ;) === adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: james_w, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [12:52] jml: can you do a db review if i don't get it done before stub goes EOD? [13:09] thanks adeuring and jml [13:13] bac, sure. [13:29] james_w, done! [13:32] jml: did you get bored? :-) [13:33] james_w, not so much bored as struck with a desire to come up with many ways of saying the same thing :) [14:03] james_w: r=me for another branch, again one minor nitpick ;) [14:03] adeuring: thanks, though the last one wasn't so minor :-) [14:04] adeuring: I thought the same thing about isinstance, but the test proved me wrong. Maybe something needs to be imported for the monkeypatch to happen? [14:06] james_w: interesting that you need to remove the proxy: IIRC zope.security.porxy or somesuch directly changes the built in isinstance function. [14:06] But anyway, if you must remove the proxy, then be it so. But perhaps gary_poster knows a bit more? [14:07] no, isinstance is not modified [14:07] jml said that you can "from zope.security.proxy import isinstance as zope_isinstance" [14:07] it's not called that [14:08] gary_poster, we have code in Launchpad that does exactly that. [14:08] at least I don't think so (and I use the one from zope.proxy) [14:08] gary_poster: thanks -- seems that I am somewhat confused... [14:08] but that's the right idea [14:08] ok, jml, fair enough. that's certainly the right idea, as I said. zope.proxy has the more general functions with which I am more familiar [14:09] gary_poster, more general functions sounds like a good idea :) [14:12] :-) well, looks like my memory is just faulty. The general functions are there that work with the core proxy stuff, but there's not a direct isinstance equivalent. (Available functions are listed in eggs/zope.proxy-3.5.0-py2.6-linux-x86_64.egg/zope/proxy/interfaces.py fwiw) [14:13] a function called "zope_isinstance" offends my naming sensibilities ;-) [14:13] so, importing from zope.security.proxy would be the roght way? [14:13] yes, adeuring [14:13] sorry for being confusing === adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [14:53] adeuring: hello again, could I get an incremental review of https://code.edge.launchpad.net/~james-w/launchpad/soyuz-factory-improvements/+merge/31890 ? [14:53] james_w: sure [14:53] oh, conflicts, I need to merge devel [15:05] done [15:05] * adeuring pulls the latest version === matsubara-afk is now known as matsubara [15:21] jml: can you do a db review for https://code.edge.launchpad.net/~bac/launchpad/db-bug-613442/+merge/31955 ? [15:22] bac, sure, after this call. [15:22] thx [15:25] james_w: again r=me. nice work! [15:25] adeuring: thanks [15:45] bac, approved. [15:45] thanks jml [15:58] adeuring: may i get a code review for https://code.edge.launchpad.net/~bac/launchpad/db-bug-613442/+merge/31955 ? [15:58] bac: sure [15:58] thanks === salgado is now known as salgado-lunch [16:17] bac: why didn't you add a column for bug tracking? [16:17] (and for code hosting, for projects) [16:18] adeuring: bug tracking and code hosting can be properly derived from other pillar data. [16:18] adeuring: it is better to have a derived attribute than a column in the db that may become out of sync === Ursinha is now known as Ursinha-lunch [16:19] bac: what about "bugs are tracked elsewhere"? [16:19] adeuring: that would be indicated by having an external bug tracker noted [16:20] adeuring: this email thread lays out much of the thinking, dense as it is: https://lists.launchpad.net/launchpad-dev/msg03952.html [16:20] bac: Ah, right. And these is at present no need to indicate this in the comuted attribute? [16:20] adeuring: i don't understand your question. [16:20] sorry, my usual typing problems... [16:21] bac: shouldn't the value "bugs are tracked elsewhere" being returned by bug_tracking_usage? (for porjects which do that) [16:22] gah, that's done [16:22] sorry, I was reading the diff for distros... [16:23] * adeuring should read the entire diff before asking pointless questions... [16:26] bac: r=me [16:27] thanks abel [16:34] jelmer_: do you have time to answer some of my questions on the branch you're reviewing? [16:42] EdwinGrubbs, sure [16:43] EdwinGrubbs: I don't see a followup from you after my last reply [16:43] jelmer_: oh, sorry, I didn't see your reply before. === adeuring 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 === salgado-lunch is now known as salgado [17:35] jml: you approved the db review of my branch. what should i do about patch number? next sequential? how do i know there is no conflict? [17:35] jml: i assume stub has a Big Chief Tablet where he coordinates the numbering === Ursinha-lunch is now known as Ursinha === deryck is now known as deryck[lunch] [17:53] bac, I don't have answers to any of your questions, I'm afraid :( [17:54] jml: well i'll pick the next and cross my fingers. (after looking at the unlanded MPs first) === sinzui changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [17:55] bac, that seems like the best approach [17:55] bac: I have a branch that move the registration slot. [17:56] sinzui: ok [17:56] sinzui: are you going to get a UI review too? [18:00] I was hoping rockstar could look at the picture and say this is agreeable to him, me, and henninge [18:00] * rockstar looks up [18:01] sinzui, what picture? [18:02] rockstar, the picture listed in https://code.launchpad.net/~sinzui/launchpad/launchpad-header-1/+merge/31975 [18:04] sinzui, ui=rockstar [18:04] sinzui, I still hate that we feel that should go at the top of the page. [18:05] yes. Maybe I will take a week off and write an alt-css that allows launchpad to get that non-sense out of the way [18:09] speaking of, is there any way to remove the "Registered by" slot for a project? Every time I visit the page for my little project Manuel I'm slightly jarred by the fact that it says "Registered 2010-01-13 by Björn Tillenius" [18:09] (he registered it to work on some branches for Manuel before I used LP) [18:13] sinzui: r=bac [18:13] thanks bac. thanks rockstar [18:14] benji: no. why do you find it objectionable? it is a true statement. [18:14] benji, no, and that's why I think it should just go away. It can be misleading. [18:15] bac, it's a true statement, yes. But it's almost worthless to put right at the top. [18:15] bac: it's strange (or misleading as rockstar says); it's odd to place such an unimportant bit of information in such a prominant place [18:16] * benji assigns rockstar as his spokesman on this topic. [18:16] bac, it has some useful information sometimes, but it's not information that should go at the top. [18:17] I can only assume that it was done that way initially to reward people for registering projects in LP, but I think we're beyond that now. [18:17] I look at who registered a project, bug, question, and mirror every day. It is not important, but it is information users actually want to know [18:17] In most cases, the registrant is the same as the owner. In other cases, it's just misleading. In a few cases, you actually need to know it. Those people can go hunting on the page for it. [18:18] Not that you can so anything about someone who reports a bug that launchpad causes flatulence in donkeys [18:18] bug, question, and mirror make sense to me, but it's not really that interesting to know that I registered the Firefox project if I don't really have anything to do with the project itself [18:19] * benji goes looking for high-profile projects that aren't yet represented in LP in a "Registered by" landgrab. [18:20] If you are looking for missing upstream information, the registrant is one of the people you want to contact [18:20] Most projects are missing information about upstream services [18:22] oh, don't get me wrong, I think the information should be recorded and visible, but I don't see why regiterer should be more prominent than homepage, owner, driver, etc. [18:23] rockstar: i updated to PG8.4 so i could cleanly make new sample data. by the time i got ready to do it, there was a new version...made with 8.3. i give up [18:24] bac, you should blame on that and see who did that. [18:26] is it me, or is PG8.4 much slower for 'make schema'? [18:26] benji, I think we all agree. The heading was design by canonical's design team to address the need of projects and people to see their identity prominently as a means to encourage adoption [18:26] * sinzui did not ask for permission to change the header [18:35] benji, I disabled about 5000 projects that were not appropriate. Most land grabs last a few hours [18:43] sinzui: wow, 5000? That's quite a few. [18:48] I have these numbers because I started writing a summary of project review. What you need to know is that I temporarily exceeded your record for linking projects to packages [19:00] sinzui: ah, thanks === deryck[lunch] is now known as deryck [19:43] Hi, bac. I have an easy one-line change to versions.cfg for your review, if I may get in line. [19:44] sure === deryck changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [sinzui, deryck] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: deryck || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [19:44] thanks, bac! The diff is large because I'm also re-enabling some reverted work. All that is new is the change to versions.cfg in the diff. [19:44] https://code.edge.launchpad.net/~deryck/launchpad/update-storm-17-add-back-dupe-changes/+merge/31983 [19:44] sinzui: your name in the queue was leftover, right. you have nothing outstanding for review? [19:45] right, but I will add it back in 30 minutes [19:45] deryck: one line or 1296 lines? [19:45] deryck: which one of those lines do you want reviewed? [19:45] should i just pick one? [19:46] i approve of line 157. it is poetry. r=bac [19:46] lol [19:47] bac, the change at line 1293 of the diff is all that is new. I'm just being lazy and adding back previous work that was reverted as I update the version of Storm. [19:47] heh [19:49] bac, I noted the actual line in scrollback if you really didn't see. Line 1293. :-) [19:49] deryck: wow. ok, it looks fine given that explanation. [19:49] bac, great, thanks. === sinzui changed the topic of #launchpad-reviews to: On call: bac || reviewing: deryck || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [21:07] bac: do you have time for a cache fix: https://code.edge.launchpad.net/~sinzui/launchpad/registry-cache-4/+merge/31998 [21:07] sinzui: sure [21:08] bac I see a bad comment. in the second test. I wait for you challenge to make it a sensible comment [21:11] bugger, I lost my bug id to [21:12] sinzui: so line 51 should be miss. is that what you're referring to? [21:12] yes [21:12] with that change it looks fine to me [21:12] r=festus [21:13] thanls [21:13] any gunsmoke fans in the house? [21:13] thanks bac [21:23] bac: I would have gotten the reference if you'd picked any other popular US series from the 70s; not into Gunsmoke [21:24] * benji has a sudden desire to watch The Rockford Files. [21:40] benji: "Who was Micah" for 20 points? [21:40] I think I need to phone a friend [21:42] no "Gunsmoke" or "The Rifleman"? [21:42] Hoss Cartwright? [21:42] Joe Friday? [21:43] not really into westerns [21:43] I'm down with Joe Friday; my son would vote for Reed and Malloy (Adam 12) [21:44] for some reason i preferred "Emergency!" [21:49] I tend to favor comedies from the 70s, most of the "action" series are pretty slow and linear by today's standards. === bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews [23:09] benji, my wife is a Rockford Files fanatic [23:11] we found out that netflix has the full run of Rockford in their online instant view thingee, so she can watch it whenever she wants. === Ursinha is now known as Ursinha-afk === matsubara is now known as matsubara-afk