| al-maisan | hello BjornT, the MP in question is here: https://code.edge.launchpad.net/~al-maisan/launchpad/ibuilder-api-cleanup-505647/+merge/17122 | 03:08 |
|---|---|---|
| === stub1 is now known as stub | ||
| al-maisan | 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 ? | 04:29 |
| rockstar | Anyone around for a review? | 06:39 |
| === 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 | ||
| gmb | henninge: Can i get a review for https://code.edge.launchpad.net/~gmb/launchpad/bug-heat-infrastructure-bug-503745/+merge/17137 please? | 11:29 |
| gmb | Fixes bug 503745 | 11:29 |
| mup | Bug #503745: Add infrastructure for calculating bug heat <story-bug-heat> <Launchpad Bugs:In Progress by gmb> <https://launchpad.net/bugs/503745> | 11:29 |
| henninge | gmb: Sure. ;) | 11:33 |
| gmb | Thanks. | 11:33 |
| henninge | gmb: That's a very brief cover letter for a 509 lines diff. Expect questions. | 11:33 |
| === 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 | ||
| gmb | henninge: Sure; I look forward to them. | 11:34 |
| henninge | :) | 11:34 |
| henninge | 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:37 |
| gmb | henninge: Yes. | 11:38 |
| henninge | gmb: I think it'd prevent confusion if you iliminated the term "hotness" from the code, don't you think so, too? | 11:39 |
| henninge | or are there other constraints? | 11:39 |
| gmb | henninge: Hmm. Probably; I was sticking with our usual policy of reflecting DB fieldnames in the interface. | 11:39 |
| gmb | henninge: No, this is brand new, so it makes no difference. | 11:40 |
| henninge | so the db fieldname was named wrongly or prematurely? | 11:40 |
| gmb | henninge: Prematurely. | 11:40 |
| gmb | henninge: I agree, consistence in the code is better. | 11:40 |
| gmb | I'll change hotness -> heat in the code. | 11:40 |
| henninge | gmb: Great! | 11:41 |
| gmb | (We can always patch the DB later to update the field name) | 11:41 |
| henninge | thanks | 11:41 |
| henninge | yup | 11:41 |
| === 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 | ||
| henninge | gmb: This line: bugs = list(getUtility(IBugSet).dangerousGetAllBugs()[start:end]) | 12:11 |
| gmb | henninge: Yes? | 12:11 |
| gmb | henninge: Hmm. That list() seems unnecessary. Don't remember why I did that... | 12:12 |
| henninge | 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 |
| gmb | Let me check to see if I can remove it. | 12:12 |
| henninge | ? | 12:12 |
| gmb | henninge: The slicing works on the iterator. | 12:12 |
| henninge | gmb: ah, cool | 12:12 |
| gmb | But the list() might be totally bogus. | 12:12 |
| henninge | gmb: You have an "if bugs:" after that for which you might need it. | 12:13 |
| henninge | gmb: But I don't like "if bugs:" anyway, or rather, our standards don't, afaik. | 12:13 |
| henninge | ;) | 12:13 |
| gmb | henninge: Hmm, yes, I suspect that's it. I'll see what happens when I remove the list(). | 12:13 |
| gmb | henninge: Right. That should be if bugs.any(). | 12:14 |
| gmb | (is not none) ;) | 12:14 |
| gmb | 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 |
| henninge | gmb: either that or calculate the length into a local variable, you need it for the logger output anyway. | 12:14 |
| henninge | I know the feeling. | 12:14 |
| gmb | henninge: Good point. | 12:14 |
| gmb | henninge: Ah; I need the list() so I can do bugs[0]. | 12:15 |
| henninge | is there no "bugs.first()" in storm? | 12:15 |
| gmb | henninge: I'm about to find out :) | 12:15 |
| gmb | henninge: There is, and it works :) | 12:18 |
| henninge | cool | 12:18 |
| * henninge just learned about test_doc.py ... | 12:22 | |
| === salgado is now known as salgado-brb | ||
| stub | 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 |
| gmb | stub: Thanks, will do. | 12:33 |
| stub | gmb: Also consider putting this in garbo-daily rather than a seperate script. | 12:33 |
| gmb | stub: Ah, good thinking. I'll look into that. | 12:34 |
| stub | 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:34 |
| gmb | Right. | 12:35 |
| gmb | stub: Right, I think I'll do that, thanks. Seems like an easy win. | 12:36 |
| mrevell | 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:41 |
| henninge | mrevell: np, but it will have to be after lunch. | 12:42 |
| mrevell | thanks henninge | 12:42 |
| henninge | mrevell: put it in the queue | 12:42 |
| henninge | please | 12:42 |
| henninge | ;) | 12:42 |
| mrevell | henninge, It's there already :) | 12:42 |
| === 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 | ||
| intellectronica | henninge, mrevell: i can review this, since you're already reviewing stuff | 12:45 |
| henninge | mrevell: it's rosetta-specific help text, right? | 12:46 |
| mrevell | 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 |
| intellectronica | right | 12:46 |
| henninge | gmb, review sent. I am off to lunch but we'll have a (short) second round later ... ;) | 13:05 |
| gmb | henninge: Cool, thanks. I'll grab some lunch too. | 13:06 |
| === henninge is now known as hennige-lunch | ||
| mrevell | thanks intellectronica | 13:12 |
| === mrevell is now known as mrevell-lunch | ||
| === stub1 is now known as stub | ||
| === salgado-brb is now known as salgado | ||
| salgado | intellectronica, care to review a one-liner for me? http://paste.ubuntu.com/355009/ | 13:37 |
| intellectronica | salgado: sure | 13:37 |
| salgado | 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 |
| intellectronica | salgado: cool. r=me | 13:38 |
| salgado | thanks intellectronica | 13:38 |
| === 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 | ||
| allenap | 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:06 |
| intellectronica | allenap: of course | 14:14 |
| gary_poster | jml: I'm looking at https://code.edge.launchpad.net/~jml/launchpad/use-testtools/+merge/16711 | 14:14 |
| gary_poster | you there? | 14:14 |
| allenap | intellectronica: Thanks :) | 14:14 |
| gary_poster | jml lines 193 and 201 look like you have checked in diff cruft | 14:15 |
| gary_poster | other than that looks fine. will comment as such. | 14:16 |
| intellectronica | allenap: why did you remove the assignment to html_data? | 14:21 |
| intellectronica | was it lint? | 14:21 |
| salgado | intellectronica, care to review another trivial one? this time a testfix: http://pastebin.ubuntu.com/355032/ | 14:22 |
| allenap | intellectronica: Yes, lint. | 14:22 |
| intellectronica | salgado: sure, let me just finish with allenap's branch first | 14:23 |
| intellectronica | allenap: r=me | 14:24 |
| salgado | 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:24 |
| allenap | intellectronica: Woohoo, thanks :) | 14:25 |
| gary_poster | salgado: great thanks | 14:25 |
| intellectronica | salgado: r=me | 14:26 |
| salgado | thanks intellectronica | 14:26 |
| bac | 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:47 |
| intellectronica | 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:48 |
| bac | intellectronica: darn, i forgot. that would just be beuno and noodles at this point. | 14:49 |
| bac | intellectronica: but please have a look if you want | 14:49 |
| bac | beuno: could you do a mentored review for a UI change? | 14:50 |
| intellectronica | bac: sure thing | 14:50 |
| bac | thanks | 14:50 |
| === ursula is now known as Ursinha | ||
| intellectronica | 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 |
| beuno | bac, sure | 14:53 |
| bac | intellectronica: ok. i'll make that change to both cases | 14:53 |
| intellectronica | bac: cool, so ui=me with that. let's see what beuno thinks | 14:53 |
| * beuno looks | 14:53 | |
| beuno | bac, I have a few comments | 14:58 |
| beuno | IRC or MP? | 14:58 |
| === salgado is now known as salgado-lunch | ||
| bac | beuno: either | 14:58 |
| bac | beuno: i need to be afk for about 15 minutes, though | 14:59 |
| beuno | IRC it is | 14:59 |
| beuno | ok | 14:59 |
| beuno | MP it is :) | 14:59 |
| hennige | mrevell: The help popups are missing CSS on my lp.dev and the spinner keeps spinning ... | 15:03 |
| hennige | mrevell: btw, I wonder if "continue" is the right labeling for the "close" button or the popup. | 15:04 |
| hennige | s/or/of/ | 15:04 |
| mrevell | 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 |
| mrevell | hennige, I'm not sure what's happening with the stylesheet there. | 15:04 |
| * hennige revs up firebug | 15:05 | |
| hennige | mrevell: 404 on reset.css, fonts.css, base.css | 15:05 |
| hennige | those are the YUI css base files. | 15:05 |
| beuno | intellectronica, bac, reviews and OOPSES! | 15:06 |
| mrevell | sinzui, I rocketfuel-branched so there shouldn't be any real divergence. As I say, the same problem shows on edge. | 15:06 |
| mrevell | sorry, hennige, not sinzui | 15:07 |
| beuno | rockstar, thumper, https://pastebin.canonical.com/26402/ | 15:07 |
| mrevell | hennige, Do you know your nick is mis-spelt? | 15:07 |
| beuno | intellectronica, bac, I can't review, the MP oopses. So: http://paste.ubuntu.com/355050/ | 15:08 |
| === hennige is now known as henninge | ||
| henninge | mrevell: didn't, thanks | 15:08 |
| intellectronica | beuno: what oopses, adding a comment to the mp? | 15:09 |
| beuno | intellectronica, a review, yes | 15:09 |
| intellectronica | beuno: let me try. worked for me with other MPs earlier | 15:10 |
| intellectronica | yep. oopses for me too :( | 15:10 |
| === matsubara is now known as matsubara-lunch | ||
| bac | beuno: thanks for the comments. i'll get cracking on them | 15:20 |
| === jamalta_ is now known as jamalta | ||
| henninge | mrevell: I don't know what the change on "https://translations.launchpad.dev/~name16" is. | 15:42 |
| * henninge reads diff abain | 15:42 | |
| bac | beuno: i assume your comment about wordiness applies to the old (http://launchpadlibrarian.net/37626290/merge-visible-email.png) and new? | 15:42 |
| henninge | again | 15:42 |
| mrevell | henninge, It's the "More about reviewing" link | 15:43 |
| henninge | mrevell: ah, cool | 15:43 |
| henninge | 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:43 |
| mrevell | henninge, Should a merge from devel fix that? | 15:44 |
| henninge | mrevell: no, because those are new files you created. | 15:44 |
| mrevell | henninge, Oh, the html files for the pop-ups? | 15:45 |
| henninge | mrevell: you should look at one of the existing help html files and see how the <head> looks in those. | 15:45 |
| henninge | right | 15:45 |
| henninge | mrevell: oh no sorry, I forgot that it's not working on edge, either. | 15:45 |
| beuno | bac, it does | 15:45 |
| mrevell | henninge, Heh, I was just typing that | 15:45 |
| henninge | mrevell: but I don't see the 404 errors on edge. | 15:46 |
| mrevell | henninge, This is odd. All I did was rocketfuel-branch, so those 404 errors are derived from devel. | 15:52 |
| leonardr | flacoste, thanks for your reviews | 15:52 |
| leonardr | i'm going through them today | 15:52 |
| flacoste | leonardr: no problem | 15:53 |
| henninge | mrevell: finally found the "more about reviewing" link - I need to be logged in as a reviewer, obviously ... ;-) | 15:53 |
| mrevell | aha :) | 15:54 |
| henninge | mrevell: ah, my error. The 404's are on edge, too. | 15:58 |
| mrevell | 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? | 15:59 |
| henninge | mrevell: yes, basically | 16:04 |
| henninge | mrevell: this is what the normal pages do: http://paste.ubuntu.com/355065/ | 16:04 |
| * mrevell looks | 16:04 | |
| henninge | mrevell: but I just tried that and it makes everything centered. | 16:04 |
| mrevell | henninge, Heh, yeah, so it does. Hmm. :) | 16:05 |
| henninge | mrevell: so the help files might also need some class added somewhere. | 16:05 |
| henninge | mrevell: which brings us back to asking sinzui if he can help us ... ;-) | 16:05 |
| mrevell | :) | 16:05 |
| henninge | mrevell: also, that line will have been generated somehow to include the correct revision number. | 16:06 |
| henninge | some TAL | 16:06 |
| === salgado-lunch is now known as salgado | ||
| henninge | mrevell: or maybe salgado can help? | 16:07 |
| mrevell | 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 |
| allenap | intellectronica: Up for another external bug tracker branch? https://code.edge.launchpad.net/~allenap/launchpad/kde-bugzilla-being-odd/+merge/17142 | 16:07 |
| henninge | mrevell: ask him here, so I can learn, too. | 16:07 |
| henninge | or on #lp-dev | 16:08 |
| mrevell | salgado, same questions as I just asked elsewhere :) | 16:08 |
| intellectronica | allenap: on the phone, but can do it in ~30m | 16:08 |
| mrevell | salgado, I'm asking because we need to somehow translate that to the help pop-ups too | 16:08 |
| allenap | intellectronica: Cool, thanks, that would be great. I'll put it on the queue; maybe henninge will get to it first. | 16:08 |
| === 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 | ||
| mrevell | salgado, Perhaps we could have a chat when you're off your call. | 16:09 |
| salgado | mrevell, sure | 16:09 |
| mrevell | thanks :) | 16:09 |
| henninge | allenap: I will look at it but I will not last much longer ... | 16:09 |
| allenap | henninge: Hehe, don't worry then. It's certainly not urgent. | 16:10 |
| henninge_ | allenap: yes, thank you. I have to stop now. | 16:36 |
| === 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 | ||
| salgado | mrevell, I'm free now | 16:51 |
| mrevell | 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 |
| mrevell | salgado, Is that something you can help with? | 16:54 |
| salgado | mrevell, why do they need to be re-worked for combo.css? | 16:55 |
| mrevell | 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:55 |
| salgado | 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 |
| mrevell | oh right | 16:57 |
| salgado | mrevell, https://edge.launchpad.net/+icing/yui/cssreset/reset.css | 16:57 |
| salgado | mrevell, so, just drop /current/build from the path | 16:57 |
| mrevell | salgado, aha, thanks :) | 16:57 |
| === matsubara-lunch is now known as matsubara | ||
| === beuno is now known as beuno-lunch | ||
| allenap | 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:06 |
| allenap | intellectronica: When I say "leave it for tomorrow", I mean I'll ask tomorrow's OCR to look at it. | 17:07 |
| intellectronica | allenap: okie doke | 17:08 |
| allenap | intellectronica: Cool. Of course, if there's no OCR I might come knocking... :) | 17:11 |
| === 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 | ||
| leonardr | 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:51 |
| thumper | beuno: please file a bug with the oops, and I'll try to get to it today | 19:56 |
| beuno | thumper, will do, thanks | 19:57 |
| === salgado is now known as salgado-afk | ||
| rockstar | al-maisan, https://code.edge.launchpad.net/~rockstar/launchpad/branch-scanner-job/+merge/17132 | 20:01 |
| === kfogel-lunch is now known as kfogel | ||
| al-maisan | 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:34 |
| jml | al-maisan, sure. | 20:35 |
| jml | al-maisan, once I'm done w/ bfiller on #launchpad | 20:36 |
| al-maisan | jml: thanking you :) | 20:36 |
| jml | al-maisan, done. approved. | 20:39 |
| === matsubara is now known as matsubara-afk | ||
| al-maisan | rockstar: https://code.edge.launchpad.net/~al-maisan/launchpad/bq-platform-columns-505725/+merge/17125 | 21:06 |
| === 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 | ||
| rockstar | al-maisan, https://code.edge.launchpad.net/~rockstar/launchpad/job-suspended-status/+merge/17173 | 22:56 |
| thumper | mwhudson: https://code.edge.launchpad.net/~thumper/launchpad/imports-urls/+merge/17172 | 23:41 |
| thumper | mwhudson: diff updated sans-conflicts | 23:48 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
