=== ursula is now known as Ursinha [03:57] https://code.edge.launchpad.net/~stub/launchpad/replication/+merge/14980 for staging and dogfood rebuild goodness. Pretty trivial. [04:15] stub: shouldn't you be closing cursors? [04:15] thumper: Nah- they close themselves when they drop out of scope and the script is in autocommit mode anyway [04:16] ok [04:16] done [06:58] trivial review: https://code.edge.launchpad.net/~thumper/launchpad/branch-visibility-fubar/+merge/14982 [07:03] thumper: is not specified in the private case really appropriate? [07:04] i guess it's consistent with our approach in general [08:14] mwhudson: yes, it is consistent with what we have [08:15] thumper: fair enough [08:15] * mwhudson approves [08:15] mwhudson: thanks [08:15] * mwhudson wonders if submitting the review is going to time out [08:15] ah, not quite === matsubara-afk is now known as matsubara [11:59] allenap: Could you do a JS review of https://code.edge.launchpad.net/~gmb/launchpad/subscribers-timeout-bug-471974/+merge/14955 for me? bac suggested I get someone to review the JS specifically. [12:00] gmb: Okay. [12:00] allenap: I'll request a review. [12:00] gmb: Cool. [12:00] AAAAGH. [12:01] * gmb hates at chromium's behaviour [12:01] allenap: requested. It doesn't say JS, but I guess you can figure that out for yourself. [12:32] gmb: Why does it try to load the portlet after the ids? [12:36] gmb: I'm going to go to lunch now, so I'll catch up with you later :) [13:08] allenap: Because it... needs to load the portlet? [13:09] I suppose we could load the portlet first, then grab the IDs before setting up the subscribe / unsubscribe links. That might make more sense. [14:02] henninge: r=me [14:03] sinzui: cheers [14:10] gmb: Ah, I meant, why after? Why not fire off the request at the same time? [14:14] flacoste: Can I get an RC on this for cherry-picking? : [14:14] flacoste: https://code.launchpad.net/~abentley/launchpad/parse-binary-2/+merge/14915 [14:18] abentley: approved [14:19] flacoste: Thanks. [14:24] allenap: Well, we need the IDs before we set up the portlet, or at least before we set up the subscribe / unsubscribe links. [14:25] allenap: So it might make more sense to load the portlet and then, when that's loaded, get the IDs before setting up the links. [14:26] gmb: If the IDs fail to load, and the portlet is loaded anyway, I assume the links will not be set up? [14:27] allenap: Correct. [14:27] allenap: In fact, we can use that to our advantage by only setting up the links if the IDs are present. [14:27] Instead of trying to set them up anyway with unpredictable results. [14:28] gmb: Sounds good. [14:28] allenap: Cool. I'll go do that now. [14:38] gmb: Instead of using a place-holder template into which you put some JSON, and which then you have to parse at client end to get the JSON back out, you could override LaunchpadView.render() to simply return some JSON. [14:41] allenap: Ooh, sweet. I'll do that, thanks. [14:43] gmb: The request.response mime type might need setting too, but it'll probably work without (and I never said that). [14:44] allenap: Hah. What to, assuming that that's what I need to do? [14:45] gmb: application/json I think. [14:45] allenap: Right, thought as much; wasn't sure about the first bit though. [14:52] abentley: talk to salgado, he also has a r-c [14:52] abentley: you should land it together [14:52] * flacoste hopes it's not too late === jamalta_ is now known as jamalta [14:52] flacoste: Sorry, it's too late. [14:54] hi allenap [14:54] bac: Hi there. [14:55] allenap: what do you think of http://pastebin.ubuntu.com/321559/ [14:58] bac: Looks good. Instead of cd'ing in and cd'ing out again, I'd run it in a sub-shell, like (cd $$subdir && bzr clean-tree ...) === salgado_ is now known as salgado [14:58] allenap: hey i'll give that a try [14:58] * bac lacks makefile-fu and even bash-fu [15:02] allenap: hey that worked: http://pastebin.ubuntu.com/321567/ [15:03] bac: The exit $$? will only exit the sub-shell I think, but if you move it outside of the braces it should work. [15:04] bac: So (cd $$subdir && bzr clean-tree --ignored --force) || exit $$?; [15:05] bac: r=me though. And, does this get called as part of a make clean on the top-level Makefile? [15:05] allenap: it does not [15:05] allenap: i think cleaning sourcecode is something we should do only rarely and on-demand [15:06] what do you think? [15:06] i just wanted a target to make it easier without a lot of fuss [15:06] bac: That's fair. [15:07] allenap: if we change our minds it's an easy fix later [15:07] bac: Yes. If we wire it in now, it probably annoys a lot of people who keep having to rebuild stuff. [15:08] allenap: and normal tree updates shouldn't need a clean and rebuild. we only experienced it this week as the 2.4->2.5 change could not trigger a rebuild [15:08] thanks for the review and improvements [15:08] bac: Yep. [15:08] bac: While you're there, do you normally reply to the "Project License Submitted" messages? [15:08] bac: As in, should I leave them alone? === salgado is now known as salgado-lunch [15:09] please continue to point them out to me [15:09] i do not regularly review the feedback queue [15:09] sinzui will often reply to them whether he is CHR or not [15:10] bac: Okay. And I've just noticed https://wiki.canonical.com/Launchpad/PolicyandProcess/CommunityHelpRotation/LicensingIssues which I can work through. [15:10] bac: Thanks. [15:10] np [15:11] I do, but I only looked once last week. I had to do some project review catchup over the weekend too === abentley1 is now known as abentley [15:21] allenap: I've updated https://code.edge.launchpad.net/~gmb/launchpad/subscribers-timeout-bug-471974/+merge/14955 with an incremental diff of the suggested changes. [15:22] gmb: Top banana. [15:33] bac: See above about the branch you reviewed yesterday. [15:34] gmb: will do [15:34] Thanks [15:35] gmb: i'll update the MP. thanks for adding the test. [15:35] bac: No worries. Working on trying to get windmill working now. [15:36] gmb: my use of "fragility" was from frustration at not being able to figure out why the icon alignment issue was there. your explanation that it was pre-existing is good. [15:37] Ah, right. [15:37] bac: Yeah, I can't figure that one out, either. That reminds me - I'll file a bug now. === EdwinGrubbs changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: - || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === matsubara is now known as matsubara-lunch [15:43] edwin, can you please review https://code.edge.launchpad.net/~leonardr/lazr.restful/double-your-enjoyment/+merge/14967 ? [15:43] leonardr: sure === EdwinGrubbs changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: leonardr || queue [-] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [15:45] gmb: That's a beaut. One final question: why do you need to do the .replace('bugs.', '') bit? [15:47] allenap: I think it's to do with the correct path to the bug. I admit, I've cribbed that from what's already there. I'll remove it, see what breaks. [15:47] gmb: Cool. [15:51] allenap: Answer: nuffink. I'll remove it. [15:51] gmb: Even cooler. [15:52] allenap, bac: Thanks. [15:58] bac: FTR, the windmill tests break on this machine, but not because of the changes I've made; the changes are tested before the failure happens and I'm seeing the same failure on other branches, so think it's a not-enough-CPU issue with this box. [15:58] I *hate* having to say "It fails, but that's okay" [15:59] gmb: that's fine, in a despicable sort of way [15:59] Yeah, I feel kinda dirty. [15:59] I'll submit it to EC2 then take a shower. [16:07] EdwinGrubbs: Will you have time for a small clean-up review? https://code.edge.launchpad.net/~allenap/launchpad/remove-striped_class/+merge/14996 [16:08] allenap: sure [16:08] EdwinGrubbs: Thanks. === allenap changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: leonardr || queue [allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === salgado-lunch is now known as salgado [16:48] leonardr: what does this do? taggedValue(LAZR_WEBSERVICE_NAME, dict(singular="contact", plural="contacts")) [16:49] edwin, that sets the human-readable singular and plural name used by lazr.restful in wadl generation [16:49] ordinarily it happens automatically when the class is generated [16:49] but since i'm not using the class generation features, i need to do it manually [16:56] leonardr: is there a reason that ContactSet.path="contact" is singular? [16:56] edwin: no, that's a mistake. i'll fix it but i probably won't be using .path anyway [17:05] leonardr: why do the ITestDataSet.get() methods take a new request parameter but the getAll() methods do not? [17:05] edwin: because get() is part of the implementation of ITraverseWithGet (which i made the classes implement instead of Traversable), and getAll() is not part of any interface [17:10] leonardr: I see that TraverseWithGet has the request parameter, but ITraverseWithGet does not. Does that need to be updated in this branch? [17:10] maybe... [17:10] edwin: yes, i'll change that while i'm at it === matsubara-lunch is now known as matsubara [17:14] leonardr: review sent === EdwinGrubbs changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: - || queue [allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [17:44] edwin, here's my incremental diff. i fixed everything we talked about and added a tiny bit more tested code [17:44] https://pastebin.canonical.com/24847/ [17:44] i also cleaned up the prose such as unfinished sentences [17:47] leonardr: looks good. r=me === matsubara is now known as matsubara-afk === salgado is now known as salgado-afk [21:04] EdwinGrubbs: can i ask for a small review, please? [21:06] intellectronica: of course [21:11] EdwinGrubbs: great. https://code.edge.launchpad.net/~intellectronica/launchpad/bugs-homepage-searchbox/+merge/15007 === intellectronica changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: - || queue [allenap, intellectronica] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [22:09] EdwinGrubbs: looks like i can get a face-to-face review from deryck here, so don't worry about it [22:29] intellectronica: is there a purpose to having a bug supervisor and security contact listed in launchpad if they are not using the launchpad bug tracker? or at least should the edit buttons be removed? [22:33] intellectronica: oops, I didn't see your last comment. nevermind. === intellectronica changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: - || queue [allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [23:14] EdwinGrubbs: got time for a review? [23:14] sidnei: how big is it? [23:15] sidnei: it looks like I will have a bunch of time between meetings tomorrow at UDS, so I might be able to do it then also. [23:16] EdwinGrubbs: the mp says 2100, but 70% of that is under 'examples' in lazr-js [23:16] EdwinGrubbs: https://code.edge.launchpad.net/~sidnei/lazr-js/combo-service/+merge/15009 [23:19] sidnei: I'll start on it now, but I probably won't finish until tomorrow. [23:19] EdwinGrubbs: im more than happy with that! [23:58] Hello intellectronica, do you mind reviewing a simple and innocuous Soyuz branch? Life at the UDS :) [23:58] *live [23:59] al-maisan: there's no such thing as a simple and iccocuous soyuz branch ... but sure, where are you? :) [23:59] al-maisan: he's sat next to me, shall I hit him? :)