=== mwhudson_ is now known as mwhudson === mup_ is now known as mup [09:05] Topic for #launchpad-reviews: on call: noodles775 || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [10:34] jtv: good morning, did you see the reply to your review email by any chance? === matsubara-afk is now known as matsubara [11:27] al-maisan: going over there now [11:28] jtv: thanks! [11:30] al-maisan: shame there's always so little time at fosdem... we should've gotten a place with internet and stay for a few extra days of buildfarm work. :) [11:30] jtv: at least a few days :) [11:30] :) [11:32] al-maisan, you are cleared to land [11:34] jtv: oh, great, thanks! [11:41] jml: could you have a look at this db change? I'm going to ask for the operative statement to be run on production as well. [11:41] no. [11:41] sorry, not for another couple of hours. [11:42] jml: I'll shop around a bit then... it's pretty minimal, and I don't see stub around [11:52] BjornT: is this something you can review? A "create index" to deal with timeouts on the Bugs front page. https://code.edge.launchpad.net/~jtv/launchpad/db-bug-518965/+merge/19091 [12:01] jtv, I saw your bug about the need for the index, but I thought we removed the secondary sort on id. I'm curious why this is still needed. [12:02] deryck: that's just what I'm trying to figure out now. The page loads in 9 seconds for me, so too short for a helpful oops but too long for human consumption. [12:03] deryck: I thought gmb said that these were distinct problems, but when I actually looked I found I was mistaken. There may be other queries with the exact same problem. [12:04] hmm... closer to 19s this time. Pretty bad. [12:06] jtv, we were having other issues yesterday, which we don't normally have, we assume because we're only hitting the master dbs currently. this could be related, perhaps? [12:06] deryck: is that still the case? Yes, that'd increase load by a lot. [12:06] jtv, yes, that is the case. [12:07] jtv, gary was reluctant to land the change to re-enable slaves with stub unavailable. [12:07] deryck: we used to ratchet down the timeout for performance weeks (no idea why we stopped doing that), and this may have the same effect. [12:09] jtv, yeah, makes sense to me. [12:10] deryck: I'm now checking out edge oopses to see whether the problem is really solved. [12:17] deryck: the answer is somewhere in the middle—this particular bottleneck has been resolved (sorry for mixing metaphors) but other queries are still slow. [12:17] I'm withdrawing my patch. [12:27] jtv, ok, cool. === mrevell is now known as mrevell-lunch === noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: - || queue [noodles775] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === mrevell-lunch is now known as mrevell [13:51] noodles775: did you say intellectronica was going to do my UI review for https://code.edge.launchpad.net/~jtv/launchpad/translationimportqueueentry-info/+merge/18963 ? [13:52] jtv: i'm not sure what the current ui review policy is, but unless it changed, i can only do a mentored review (which i'll be happy to do) [13:52] jtv: nope, I said that it'd be good to check with sinzui as afaik he's wanting to build up ui reviews. [13:52] noodles775: ah, so I remembered the wrong person [13:52] If he doesn't want it, I can do it of course. [13:52] intellectronica: sorry to have bothered you [13:52] I can do the review in an hour [13:52] noodles775: I think it's a good mentoring review [13:52] no worries [13:52] sinzui: that'd be great, thanks [13:53] noodles775: has one ahead of your i think [13:56] well I think jtv's is older than mine, so I'm happy to wait ;) [13:57] mine's about a day and a half old [14:10] hey noodles775, wanna do another UI pre-imp chat? It's a very small matter. [14:16] mrevell: sure, call or chat? [14:16] noodles775, Chat'll be fine [14:16] k === salgado is now known as salgado-lunch === matsubara is now known as matsubara-lunch [15:28] rockstar, when you come in, hopefully the last lazr.restful multiversion branch for a while [15:28] https://code.edge.launchpad.net/~leonardr/lazr.restful/bump-version/+merge/19107 [15:29] leonardr, I'm actually composing the "holy crap I'm still sick" email. [15:29] rockstar: want me to give it to someone else? [15:29] leonardr, yes please. [15:29] noodles775, can you take https://code.edge.launchpad.net/~leonardr/lazr.restful/bump-version/+merge/19107 ? [15:36] leonardr: I'm just on the phone, but I'll take a look as soon as I'm off. [15:47] Hello noodles775 [15:48] When you have a moment, I wanted to ask you for some UI advice. At UDS, deryck and I identified various bits of the bugs interface that could benefit from a help pop-up. [15:48] yep [15:48] noodles775, One was the tags listing, which is also an ajax editing form. [15:49] I was thinking about adding a (? just beside the pencil icon (or "Add tags" link, if there are no tags). It looks a little, well, spare. So I wondered about adding the helping to the "Tags" label. I'm not sure. Any thoughts? [15:50] * noodles775 scans for other places where we've done something similar... [15:52] mrevell: so on the PPA index page, we have things like (in the context of uploading packages): "(Read about uploading)", but the page itself isn't as full as the bugs page... [15:52] noodles775, Yeah, and I think a full text link would detract from the tags themselves. === deryck is now known as deryck[lunch] [15:53] yep..., but that doesn't leave you many options? [15:54] Nope, hence this chat :) [15:55] We do have: https://edge.launchpad.net/@@/maybe on the +graphics page. [15:55] It looks much nicer than a text ?. [15:55] noodles775, Yeah, that's probably a good option. I prefer help links that include some context but not at the expense of cluttering up the page. I'll use that and see how it looks. Thanks. [15:56] np. === matsubara-lunch is now known as matsubara [16:02] leonardr: on line 45 of the MP diff, is this really the earliest version, or the earliest *active* version? (which line 47 seems to imply?) [16:03] noodles775: inactive versions don't exist in the lazr.restful installation, only in peoples' memories, so the concepts are equivalent [16:03] i can change the wording [16:03] leonardr: in which case, why can len(config.active_versions) == 0, but we still have an earliest_version? or is that what you'll re-word? [16:04] noodles775: it's kind of confusing but i don't want to change the interface becuase that'll cause backward incompatibility problems [16:04] there are two fields in the interface [16:04] active_versions and latest_version_uri_prefix [16:04] active_versions is a list, the other is a stirng [16:05] lazr.restful serves web services for all of those strings [16:05] so if active_versions is =['beta', '1.0'] and latest_version_uri_prefix is 'devel' there are three versions being served [16:05] if active_versions is [] and latest_version_uri_prefix is 'beta' then there is one version [16:05] though i don't recommend doing that [16:06] So if active_versions is =['beta', '1.0'] and latest_version_uri_prefix is 'devel', why are we evaluating earliest_version = 'beta', is that right? [16:06] leonardr: ^^ [16:07] noodles775: i'm having trouble parsing that sentence, but i can tell you that in that case earliest_version is 'beta' [16:07] latest_version_uri_prefix can only be the earliest version if active_versions is empty [16:07] OK, as the code implies, I'm just confused by the names. [16:08] noodles775: i'm kind of leaning towards treating the last item in active_versions as latest_version_uri_prefix [16:08] but that would be a separate job === salgado-lunch is now known as salgado [16:09] Right, that would make sense. (do you mind adding an XXX?) [16:09] sure [16:11] leonardr: just a style question, is the comment on the empty line 69 intentional? [16:11] yeah, it's like a para break. i don't know if we allow that though [16:12] Yeah, I think a blank line (without the comment) serves just as well, but if the style guide doesn't say either way, whatever you prefer :) [16:12] style guide doesn't mention it [16:13] if i see a blank line i think the first comment was talking about the blank line and the second comment is different [16:13] leonardr: s/marker_interface/version_marker on line 72 [16:13] sure [16:20] noodles775: i might as well make that latest_version_uri_prefix change because i haven't actually released a version that uses it yet. if i get rid of it now there will be no backwards compatibility [16:20] problem [16:20] leonardr: great [16:20] Two other small things: [16:21] See if you think it's worth creating a helper for the repeats of line 145-146 - the helper could even print out the ordered list items? [16:22] s/repeats/repititions sorry, bad day. [16:22] And s/an/a on line 180. [16:26] noodles775: i thought about putting that 145-146 stuff in the big helper method but decided it was easier to read if i spelled it out every time [16:27] leonardr: OK. [16:28] leonardr: another typo, s/in/In on 138 [16:29] noodles775, fixed everything so far [16:31] leonardr: Great, I think the text for versions 3.0 is incorrect? The method has changed to by_value? [16:31] leonardr: and with that, r=me. [16:31] yes, you're right [16:31] noodles775: if you can move on to https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/multiversion/+merge/19110 i'll work on getting rid of latest_version_uri_prefix [16:32] leonardr: sorry, I need to leave asap now, but I've got one more question for the this MP: [16:32] ok [16:32] Is the change in tales.py tested? [16:33] I can't see an example where we we've removed an op in a later version? [16:34] noodles775: i don't have details off the top of my head, but it is tested in that the wadl.txt test will fail if you remove the code [16:34] noodles775: actually i can add a quick test that proves it works [16:35] OK, great. I'm out of here... 'night! [16:37] can i get someone else interested in the web service to review some more branches? maybe sinzui? [16:38] on call: - || reviewing: - || queue [noodles775] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === noodles775 changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [noodles775] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [16:39] leonardr: I have reviews ahead of you, but I can commit to doing it in a few hours [16:40] sinzui: ok, let me try to find someone else. EdwinGrubbs? gary? intellectronica? [16:40] leonardr: you're asking for a review? [16:40] intellectronica: yes, if oyu have the time [16:40] how much work is it? [16:40] not much if you only want to do one [16:40] i have a couple, but i'm taking reviewers as i can find them [16:40] oright, i'll do one then [16:41] intellectronica: https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/multiversion/+merge/19110 [16:42] leonardr: the diff looks fine to me [16:43] cool [16:43] leonardr: I'll have some time late afternoon [16:43] leonardr: i wonder if it would be good to extract the code that deals with building the URL to another method, so that a specialized class can override it? [16:44] i mean if an implementation needs to build the URL using a rule other than base + '/' + version [16:44] intellectronica: i think yagni. if you want that you can subclass the client class and put your implementation in the constructor before calling the superconstructor [16:44] fair enough. r=me === deryck[lunch] is now known as deryck [18:20] flacoste, maybe you want to review the branch i chatted with you about earlier? [18:20] shouldn't take long [18:20] https://code.edge.launchpad.net/~leonardr/lazr.restful/no-latest-version/+merge/19122 [18:28] or gary or sinzui, if you're free by now [18:29] leonardr, I'll take it [18:29] gary: link above [18:30] ack, looking [18:33] leonardr: nice. approved. [18:33] gary, thanks [19:20] sinzui: do you think you'll get around to that UI review? [19:20] there's a link to some screenshots in my last comment [19:20] jtv: I am just finishing the one before you [19:20] cool [19:53] jtv: ping [19:53] sinzui: hi [19:53] sinzui: there are some rough edges, I know; I'm at your disposal [19:53] jtv: your edit links (imports/2) do not work in webkit. [19:53] I can see the edit icons in firefox, but not epiphany [19:54] sinzui: that's a known bug; I already submitted a fix to lazr-js [19:54] fab. I can do the review now that I know where the link to your changes are [19:54] (also, not a big problem here given how few people use this form) [19:55] sinzui: one of those rough edges... I put the extra information at the bottom of the page, below the form buttons. I'm not sure if that's acceptable or not. [19:56] There are examples like that already [19:57] phew :) [19:57] the other thing is that the first
  • of the