[03:08] hello BjornT, the MP in question is here: https://code.edge.launchpad.net/~al-maisan/launchpad/ibuilder-api-cleanup-505647/+merge/17122 === stub1 is now known as stub [04:29] hello jml and stub, could you please review the schema patch proposed here: https://code.edge.launchpad.net/~al-maisan/launchpad/bq-platform-columns-505725/+merge/17125 ? [06:39] Anyone around for a review? === henninge changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [11:29] henninge: Can i get a review for https://code.edge.launchpad.net/~gmb/launchpad/bug-heat-infrastructure-bug-503745/+merge/17137 please? [11:29] Fixes bug 503745 [11:29] Bug #503745: Add infrastructure for calculating bug heat [11:33] gmb: Sure. ;) [11:33] Thanks. [11:33] gmb: That's a very brief cover letter for a 509 lines diff. Expect questions. === henninge changed the topic of #launchpad-reviews to: on-call: henninge || reviewing: gmb || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [11:34] henninge: Sure; I look forward to them. [11:34] :) [11:37] gmb: First thing I notice: "heat" and "hotness" seem to be the same thing. From the ml discussion I saw, I think "heat" is the agreed term, isn't it? [11:38] henninge: Yes. [11:39] gmb: I think it'd prevent confusion if you iliminated the term "hotness" from the code, don't you think so, too? [11:39] or are there other constraints? [11:39] henninge: Hmm. Probably; I was sticking with our usual policy of reflecting DB fieldnames in the interface. [11:40] henninge: No, this is brand new, so it makes no difference. [11:40] so the db fieldname was named wrongly or prematurely? [11:40] henninge: Prematurely. [11:40] henninge: I agree, consistence in the code is better. [11:40] I'll change hotness -> heat in the code. [11:41] gmb: Great! [11:41] (We can always patch the DB later to update the field name) [11:41] thanks [11:41] yup === intellectronica changed the topic of #launchpad-reviews to: on-call: henninge, intellectronica || reviewing: gmb || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [12:11] gmb: This line: bugs = list(getUtility(IBugSet).dangerousGetAllBugs()[start:end]) [12:11] henninge: Yes? [12:12] henninge: Hmm. That list() seems unnecessary. Don't remember why I did that... [12:12] gmb: I am not sure how storm does this. Are you sure you are not querying all bugs each time or does the slicing work on the storm iterator. [12:12] Let me check to see if I can remove it. [12:12] ? [12:12] henninge: The slicing works on the iterator. [12:12] gmb: ah, cool [12:12] But the list() might be totally bogus. [12:13] gmb: You have an "if bugs:" after that for which you might need it. [12:13] gmb: But I don't like "if bugs:" anyway, or rather, our standards don't, afaik. [12:13] ;) [12:13] henninge: Hmm, yes, I suspect that's it. I'll see what happens when I remove the list(). [12:14] henninge: Right. That should be if bugs.any(). [12:14] (is not none) ;) [12:14] henninge: I cribbed some of this from the bugtask targetnamecache updater work, so I think this is a copy-paste error on my part. [12:14] gmb: either that or calculate the length into a local variable, you need it for the logger output anyway. [12:14] I know the feeling. [12:14] henninge: Good point. [12:15] henninge: Ah; I need the list() so I can do bugs[0]. [12:15] is there no "bugs.first()" in storm? [12:15] henninge: I'm about to find out :) [12:18] henninge: There is, and it works :) [12:18] cool [12:22] * henninge just learned about test_doc.py ... === salgado is now known as salgado-brb [12:33] gmb: Just added a comment to that MP to use DBLoopTuner instead of LoopTuner - its in the same file and you should use DBLoopTuner whenever doing bulk database updates. [12:33] stub: Thanks, will do. [12:33] gmb: Also consider putting this in garbo-daily rather than a seperate script. [12:34] stub: Ah, good thinking. I'll look into that. [12:34] You get some testing infrastructure for free and our daily jobs get run serially for best throughput and least pain. Should drop in easily enough since it is a tunable loop. [12:35] Right. [12:36] stub: Right, I think I'll do that, thanks. Seems like an easy win. [12:41] Hey henninge, any chance of a review on https://code.edge.launchpad.net/~matthew.revell/launchpad/translations-help-10.1/+merge/17032 when you have a moment? [12:42] mrevell: np, but it will have to be after lunch. [12:42] thanks henninge [12:42] mrevell: put it in the queue [12:42] please [12:42] ;) [12:42] henninge, It's there already :) === henninge changed the topic of #launchpad-reviews to: on-call: henninge, intellectronica || reviewing: gmb || queue [mrevell] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [12:45] henninge, mrevell: i can review this, since you're already reviewing stuff [12:46] mrevell: it's rosetta-specific help text, right? [12:46] thanks intellectronica, although henninge if you have time to look at it too I'd like your view from a Translations-guy PoV. [12:46] right [13:05] gmb, review sent. I am off to lunch but we'll have a (short) second round later ... ;) [13:06] henninge: Cool, thanks. I'll grab some lunch too. === henninge is now known as hennige-lunch [13:12] thanks intellectronica === mrevell is now known as mrevell-lunch === stub1 is now known as stub === salgado-brb is now known as salgado [13:37] intellectronica, care to review a one-liner for me? http://paste.ubuntu.com/355009/ [13:37] salgado: sure [13:38] we currently have these two branches for pygettextpo, but we should be using only the ~launchpad-pqm one. I'll delete the other one once this fix lands [13:38] salgado: cool. r=me [13:38] thanks intellectronica === hennige-lunch is now known as hennige === mrevell-lunch is now known as mrevell === hennige changed the topic of #launchpad-reviews to: on-call: henninge, intellectronica || reviewing: mrevell, - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === danilo__ is now known as danilos [14:06] intellectronica: Would you be able to review an external bug tracker branch for me? https://code.edge.launchpad.net/~allenap/launchpad/trac-sniffing-broken-bug-504310/+merge/17139 [14:14] allenap: of course [14:14] jml: I'm looking at https://code.edge.launchpad.net/~jml/launchpad/use-testtools/+merge/16711 [14:14] you there? [14:14] intellectronica: Thanks :) [14:15] jml lines 193 and 201 look like you have checked in diff cruft [14:16] other than that looks fine. will comment as such. [14:21] allenap: why did you remove the assignment to html_data? [14:21] was it lint? [14:22] intellectronica, care to review another trivial one? this time a testfix: http://pastebin.ubuntu.com/355032/ [14:22] intellectronica: Yes, lint. [14:23] salgado: sure, let me just finish with allenap's branch first [14:24] allenap: r=me [14:24] gary_poster, I was going to fix that in my testfix branch, but I don't see the conflict markers in that file, so I think jml solved the conflict before landing [14:25] intellectronica: Woohoo, thanks :) [14:25] salgado: great thanks [14:26] salgado: r=me [14:26] thanks intellectronica [14:47] intellectronica: can i have a UI review? https://code.edge.launchpad.net/~bac/launchpad/bug-419930/+merge/17015 note there are screenshots attached on the bug. [14:48] bac: sure, though note that i'm not a graduated ui reviewer. you will require an additional ui review after me (unless it's really trivial) [14:49] intellectronica: darn, i forgot. that would just be beuno and noodles at this point. [14:49] intellectronica: but please have a look if you want [14:50] beuno: could you do a mentored review for a UI change? [14:50] bac: sure thing [14:50] thanks === ursula is now known as Ursinha [14:53] bac: looks good. one thing i think can be improved is if we used the common format '[Merge accounts] or _Cancel_', instead of 'Submit' [14:53] bac, sure [14:53] intellectronica: ok. i'll make that change to both cases [14:53] bac: cool, so ui=me with that. let's see what beuno thinks [14:53] * beuno looks [14:58] bac, I have a few comments [14:58] IRC or MP? === salgado is now known as salgado-lunch [14:58] beuno: either [14:59] beuno: i need to be afk for about 15 minutes, though [14:59] IRC it is [14:59] ok [14:59] MP it is :) [15:03] mrevell: The help popups are missing CSS on my lp.dev and the spinner keeps spinning ... [15:04] mrevell: btw, I wonder if "continue" is the right labeling for the "close" button or the popup. [15:04] s/or/of/ [15:04] hennige, Hmm, that seems to happen on the live site too. As for the "Close" button, I have a bug for that and will fix it this cycle. [15:04] hennige, I'm not sure what's happening with the stylesheet there. [15:05] * hennige revs up firebug [15:05] mrevell: 404 on reset.css, fonts.css, base.css [15:05] those are the YUI css base files. [15:06] intellectronica, bac, reviews and OOPSES! [15:06] sinzui, I rocketfuel-branched so there shouldn't be any real divergence. As I say, the same problem shows on edge. [15:07] sorry, hennige, not sinzui [15:07] rockstar, thumper, https://pastebin.canonical.com/26402/ [15:07] hennige, Do you know your nick is mis-spelt? [15:08] intellectronica, bac, I can't review, the MP oopses. So: http://paste.ubuntu.com/355050/ === hennige is now known as henninge [15:08] mrevell: didn't, thanks [15:09] beuno: what oopses, adding a comment to the mp? [15:09] intellectronica, a review, yes [15:10] beuno: let me try. worked for me with other MPs earlier [15:10] yep. oopses for me too :( === matsubara is now known as matsubara-lunch [15:20] beuno: thanks for the comments. i'll get cracking on them === jamalta_ is now known as jamalta [15:42] mrevell: I don't know what the change on "https://translations.launchpad.dev/~name16" is. [15:42] * henninge reads diff abain [15:42] beuno: i assume your comment about wordiness applies to the old (http://launchpadlibrarian.net/37626290/merge-visible-email.png) and new? [15:42] again [15:43] henninge, It's the "More about reviewing" link [15:43] mrevell: ah, cool [15:43] mrevell: looking at the diff (again), I saw that the css files in question are being included in each html page. That boiler plate code has probably changed (or is gone). [15:44] henninge, Should a merge from devel fix that? [15:44] mrevell: no, because those are new files you created. [15:45] henninge, Oh, the html files for the pop-ups? [15:45] mrevell: you should look at one of the existing help html files and see how the looks in those. [15:45] right [15:45] mrevell: oh no sorry, I forgot that it's not working on edge, either. [15:45] bac, it does [15:45] henninge, Heh, I was just typing that [15:46] mrevell: but I don't see the 404 errors on edge. [15:52] henninge, This is odd. All I did was rocketfuel-branch, so those 404 errors are derived from devel. [15:52] flacoste, thanks for your reviews [15:52] i'm going through them today [15:53] leonardr: no problem [15:53] mrevell: finally found the "more about reviewing" link - I need to be logged in as a reviewer, obviously ... ;-) [15:54] aha :) [15:58] mrevell: ah, my error. The 404's are on edge, too. [15:59] henninge, So, I'm not sure what needs to be done about that. Do I need to change the help pop-ups to reference combo.css? [16:04] mrevell: yes, basically [16:04] mrevell: this is what the normal pages do: http://paste.ubuntu.com/355065/ [16:04] * mrevell looks [16:04] mrevell: but I just tried that and it makes everything centered. [16:05] henninge, Heh, yeah, so it does. Hmm. :) [16:05] mrevell: so the help files might also need some class added somewhere. [16:05] mrevell: which brings us back to asking sinzui if he can help us ... ;-) [16:05] :) [16:06] mrevell: also, that line will have been generated somehow to include the correct revision number. [16:06] some TAL === salgado-lunch is now known as salgado [16:07] mrevell: or maybe salgado can help? [16:07] henninge, Ah, that's a shame as they've been nice and simple HTML up until now. I'll ask Mr Salgado :) Cheers henninge [16:07] intellectronica: Up for another external bug tracker branch? https://code.edge.launchpad.net/~allenap/launchpad/kde-bugzilla-being-odd/+merge/17142 [16:07] mrevell: ask him here, so I can learn, too. [16:08] or on #lp-dev [16:08] salgado, same questions as I just asked elsewhere :) [16:08] allenap: on the phone, but can do it in ~30m [16:08] salgado, I'm asking because we need to somehow translate that to the help pop-ups too [16:08] intellectronica: Cool, thanks, that would be great. I'll put it on the queue; maybe henninge will get to it first. === allenap changed the topic of #launchpad-reviews to: on-call: henninge, intellectronica || reviewing: mrevell, - || queue [allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [16:09] salgado, Perhaps we could have a chat when you're off your call. [16:09] mrevell, sure [16:09] thanks :) [16:09] allenap: I will look at it but I will not last much longer ... [16:10] henninge: Hehe, don't worry then. It's certainly not urgent. [16:36] allenap: yes, thank you. I have to stop now. === henninge_ changed the topic of #launchpad-reviews to: on-call: intellectronica || reviewing: - || queue [allenap] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [16:51] mrevell, I'm free now [16:54] hi salgado. The help pop-ups (as an example lib/lp/translations/help/translation-groups.html) need to be reworked for combo.css. henning mentioned that they'll probably need some TAL now to update the version number. [16:54] salgado, Is that something you can help with? [16:55] mrevell, why do they need to be re-worked for combo.css? [16:55] salgado, Because I believe the css files the pop-ups rely on have been deleted. Take a look at https://edge.launchpad.net/+help/openid.html for an example [16:57] mrevell, oh, no, that's not because of my combo.css work -- it's because the latest lazr-js moved these files to a different place [16:57] oh right [16:57] mrevell, https://edge.launchpad.net/+icing/yui/cssreset/reset.css [16:57] mrevell, so, just drop /current/build from the path [16:57] salgado, aha, thanks :) === matsubara-lunch is now known as matsubara === beuno is now known as beuno-lunch [17:06] intellectronica: I've got to head off soon to pick up Robin. I'll be back on at about 2030, but I assume you'll be gone by then. It's not urgent, so please leave it for tomorrow if you'd rather chat about it. [17:07] intellectronica: When I say "leave it for tomorrow", I mean I'll ask tomorrow's OCR to look at it. [17:08] allenap: okie doke [17:11] intellectronica: Cool. Of course, if there's no OCR I might come knocking... :) === deryck is now known as deryck[lunch] === gary_poster is now known as gary-lunch === intellectronica changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews === kfogel is now known as kfogel-lunch === beuno-lunch is now known as beuno === deryck[lunch] is now known as deryck === gary-lunch is now known as gary_poster === EdwinGrubbs is now known as Edwin-lunch [19:51] flacoste, would you take a look at my most recent comment at https://code.edge.launchpad.net/~leonardr/lazr.restful/multiversion-collection/+merge/16924 and let me know if i'm missing anything? [19:56] beuno: please file a bug with the oops, and I'll try to get to it today [19:57] thumper, will do, thanks === salgado is now known as salgado-afk [20:01] al-maisan, https://code.edge.launchpad.net/~rockstar/launchpad/branch-scanner-job/+merge/17132 === kfogel-lunch is now known as kfogel [20:34] jml: could you please review the schema patch we discussed yesterday (https://code.edge.launchpad.net/~al-maisan/launchpad/bq-platform-columns-505725/+merge/17125) ? [20:35] al-maisan, sure. [20:36] al-maisan, once I'm done w/ bfiller on #launchpad [20:36] jml: thanking you :) [20:39] al-maisan, done. approved. === matsubara is now known as matsubara-afk [21:06] rockstar: https://code.edge.launchpad.net/~al-maisan/launchpad/bq-platform-columns-505725/+merge/17125 === Edwin-lunch is now known as EdwinGrubbs === jtv changed the topic of #launchpad-reviews to: on-call: - || reviewing: - || queue [jtv - http://tinyurl.com/y87el9a] || This channel is logged: http://irclogs.ubuntu.com || https://code.edge.launchpad.net/launchpad/+activereviews [22:56] al-maisan, https://code.edge.launchpad.net/~rockstar/launchpad/job-suspended-status/+merge/17173 [23:41] mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/imports-urls/+merge/17172 [23:48] mwhudson: diff updated sans-conflicts