/srv/irclogs.ubuntu.com/2010/08/10/#launchpad-reviews.txt

=== Ursinha is now known as Ursinha-afk
jtvsay henninge, aren't you ocr today?08:34
StevenKNo, it's gmb08:35
* StevenK already checked08:35
=== henninge_ is now known as henninge
jtvhenninge: I just put that API simplification branch up for review.08:57
henningelemme look08:58
henningejtv: looks good. Shouldn't it have a bug?09:03
jtvhenninge: well, it's a feature-branch task so I'm not sure we need to go to those lengths.  And it's just split off from, depending on your point of view, yesterday's branch or the job I'm working on now.09:04
henningeok09:04
StevenKgmb: O hai, Mr OCR!09:14
gmbStevenK, Morning. If you've got something that needs review bung it in the queue and I'll take a look in about an hour's time. Still catching up on emails.09:24
lifelessgmb: hey09:24
lifelessgmb: I have an _easy_ fix for you09:24
lifelesshttps://bugs.edge.launchpad.net/malone/+bug/61564409:25
_mup_Bug #615644: BugTask:+distrotask timeout on HEAT lookup <timeout> <Launchpad Bugs:Triaged> <https://launchpad.net/bugs/615644>09:25
gmblifeless, Easy fix for me to create a branch for, or is there already a branch that needs reviewing?09:26
lifelessgmb: easy fix for a sparkling young fella like you to make a branch for09:27
gmblifeless, Righto, I shall take care of it presently.09:29
StevenKgmb: Maybe you should suggest lifeless reviews the branch :-)09:29
lifelessgmb: just needs a ordered.first() turned into a .max()09:31
gmblifeless, Ah, even better.09:31
gmblifeless, Do you know, or can you point me to a way of finding out, whereabouts that query is getting issued? I'm struggling to find it (though this may be a case of E_NOT_ENOUGH_CAFFEINE).09:53
lifelessgmb: well, have a look at the linked oops09:54
lifelessgmb: as for trapping it, uhm, I've been strugglying with that too09:54
lifelessputting a hook in the thing tha tappaends to get_request_statements might be best09:54
gmblifeless, Ah, I think I may have found it. I was looking for Storm but maybe the query is pure SQL.09:54
gmblifeless, Yep, it's HasBugHeatMixin.recalculateBugHeatCache() that's the culprit. lib/lp/bugs/model/bugtarget.py:23909:56
gmblifeless, https://code.edge.launchpad.net/~gmb/launchpad/bug-615644/+merge/32175 if you're interested.10:12
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== StevenK changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
StevenKgmb: *cough* :-)10:18
gmbStevenK, Righto. Good job you put it in the queue otherwise (/me looks at +activereviews) jelmer was first up :)10:19
gmbStevenK, That could have done with a more detailed cover letter. See https://dev.launchpad.net/CoverLetters for templates. So, expect questions.10:22
gmb(Although I'm kidding a bit because of the nature of the branch)10:23
gmb\0/ for killing doctests10:23
StevenKgmb: I seriously tried three times to write a better cover letter and come up with writers block.10:23
gmbStevenK, Understandable.10:24
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
StevenKgmb: I'm starting to cook dinner, so I'll be afk and such like10:31
gmbStevenK, Okidoke.10:33
henningejtv1: you mp is not emitting emails again. Could you investigate that or have it investigated, please?10:34
jtv1henninge: grrr.  OK.10:35
StevenKgmb: Are you open to bribes of pasta bake? :-)10:35
jtv1henninge: btw I did get the email where I requested the review10:36
henningejtv1: but there is none on the list.10:36
henningejtv1: also, when I claim it, I should get one... let me check.10:36
gmbStevenK, Pasta bake may or may not be sufficient for your branch to land without oversight (see extended reviewing guidlines, subsection 1, "Bribes of food, money or sundry electronic items")10:37
StevenKBwahaha10:38
jtv1henninge: do you normally get them when you propose for merging into the recife branch?  I mean, the emails to the list aren't tied to specific LP branches?10:38
henningeoh, I did not think about that ...10:39
henningejtv1: I just remeber OCRs picking up on recife branches so I always thought they had been notified.10:40
henningebut maybe it's just because they appear in +activereviews.10:40
jtv1henninge: sounds more likely… I don't think anyone follows the lp-reviews list for this anymore.10:40
henningejtv1: just replying to it by makes it much more easier to comment on the diff, though ...10:41
jtv1henninge: yes, it does… but for now I think I'll just say that I've investigated and found this as the most likely cause.  :-)10:41
henningejtv1: well done.10:55
henninge;-)10:55
* jtv1 salutes to the sound of heroic trumpets10:55
henningejtv1: review about to be sent, r=me with a few suggestions.10:55
jtv1henninge: great, thanks!10:55
* henninge lunches10:58
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
StevenKgmb: All of the checks in test_initialise() are ripped straight from the doctest, and are checking that source and binary publications were copied correctly.11:14
gmbStevenK, Okay, that's fine. Please add comments explaining that (you don't need to comment the individual asserts, though)11:25
StevenKgmb: Yup, was going to do so tomorrow morning11:25
gmbCool.11:26
=== noodles775 changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
noodles775Hi gmb, when you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/561586-move-js-to-lp-app/+merge/3218011:58
noodles775I'll take a look at StevenK's branch now too.11:58
gmbnoodles775, Thanks muchly. Looking at your MP now.12:00
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
wgrantgmb: Hi. https://code.edge.launchpad.net/~wgrant/launchpad/bug-590708-getBuildsByArchIds-timeout/+merge/32181 is a nice simple RC branch, when you have time.12:03
=== wgrant changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: noodles775 || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
noodles775^^^ can take priority over mine which is not RC.12:03
gmbnoodles775, r=me.12:11
gmbwgrant, Looking now12:11
noodles775Thanks gmb12:11
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
wgrantgmb, noodles775: Thanks.12:12
gmbwgrant, Nice solution :)12:13
gmbr=me12:13
* gmb lunches before anything else comes up12:14
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
wgrantgmb: Ahem.12:14
wgrantThanks.12:14
wgrantDo I request an 'rc', or a 'release-critical'?12:15
StevenKThe latter12:15
wgrantD: There are two Julian Edwardses, both with hidden email addresses.12:16
bigjoolsI am julian-edwards12:17
bigjoolsI guess I should expose my canonical one :)12:17
wgrantbigjools: I know, but the UI doesn't tell me the username.12:17
wgrantIt just lists two 'Julian Edwards <email address hidden>'12:17
wgrantSo I have to use Firebug to work out who is who, I guess.12:18
bigjoolshmmm that's bad12:18
wgrantIt should probably show the username, yeah.12:18
bigjools+editemails doesn't let me change privacy. WTF do I go12:18
wgrantThe might be on +edit, IIRC. But you can't just expose your primary; you have to expose all :(12:19
bigjools:/12:19
wgrantYes.12:20
* StevenK subsribes bigjools to 1,000,000 mailing lists12:20
StevenKsubscribes, even12:20
* wgrant gives up and uses the non-AJAX form.12:21
bigjoolsso "Hide my email addresses from other Launchpad users" really ought to be on the +editemails form12:21
bigjoolssigh12:21
bigjoolsbug 387774 it seems12:22
_mup_Bug #387774: Hiding email addresses should be done from the email settings page <Launchpad Registry:Triaged> <https://launchpad.net/bugs/387774>12:22
wgrantHah, I reported it.12:22
bigjoolslol12:22
StevenKThen fix it?12:22
* StevenK hides12:23
bigjoolsCurtis Hovey  wrote on 2009-06-16:  "We can fix this in July and August."12:23
bigjoolslmao12:23
wgrantIt was more than a month before the code was released! I am in the clear.12:23
wgrantbigjools: He didn't state a year...12:23
bigjoolsexactly :)12:23
StevenKBwahaha12:23
StevenKwgrant: Fix it now?12:23
wgrantSilence!12:23
StevenKYou have the code ...12:23
* StevenK baits wgrant via phone12:24
wgrantUhoh.12:24
bigjoolsI filed bug 61580912:25
_mup_Bug #615809: I should be able to selectively hide email addresses <Launchpad Registry:New> <https://launchpad.net/bugs/615809>12:25
* wgrant me-toos.12:26
bigjoolsI don't expect it to get fixed soon, Curtis has even more bugs than me12:28
wgrantbigjools: Anyway, can you RC-stamp that branch?12:35
bigjoolswgrant: done.  let's ask gmb nicely to land it :)12:41
wgrantbigjools: Thanks.12:47
wgrantgmb: Can you please land it?12:47
=== jtv1 is now known as jtv
=== matsubara-afk is now known as matsubara
=== Ursinha-afk is now known as Ursinha
gmbwgrant, Yes, sure.13:22
wgrantgmb: Thanks.13:23
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbbigjools, On a similar note, can I get an RC for this: https://code.edge.launchpad.net/~gmb/launchpad/bug-615644/+merge/3217513:24
gmbTimeout fix.13:24
* bigjools looks13:24
bigjoolsgmb: nice change, I'm glad we have stub :)13:25
gmbbigjools, Aren't we all :)13:26
bigjoolsgmb: FWIW your bug is unassigned and not in-progress13:27
bigjoolsrc=me anyway13:27
gmbbigjools, Ooops. And there's no Kanban card for it either. This is what I get for accepting on-the-fly assignments. Thanks.13:27
bigjools:)13:27
leonardrgmb, can you take a look at https://code.edge.launchpad.net/~leonardr/lazr.restful/benji-updated-size-link/+merge/32187 ?13:36
gmbSure13:36
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbleonardr, Woah, 1233 lines? Oh, wait. I see. nm13:39
gmbleonardr, r=me.13:50
leonardrgmb, thanks13:50
gmbnp13:50
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbjames_w, Is https://code.edge.launchpad.net/~james-w/launchpad/archive-collection/+merge/31499 in a reviewable state?14:48
gmbOr is it WIP after your conversation with jml?14:48
james_wgmb: WIP, sorry, just changed the status14:49
gmbjames_w, Cool, thanks.14:49
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== leonardr changed the topic of #launchpad-reviews to: Today's trained review ape: gmb, leonardr || reviewing: jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jelmergmb: Thanks!15:10
=== gmb changed the topic of #launchpad-reviews to: Today's trained review ape: gmb || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbjelmer, np15:19
* gmb -> bbiab15:19
=== gmb changed the topic of #launchpad-reviews to: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== deryck is now known as deryck[lunch]
=== salgado is now known as salgado-lunch
=== Ursinha is now known as Ursinha-afk
=== deryck[lunch] is now known as deryck
=== Ursinha-afk is now known as Ursinha
=== leonardr changed the topic of #launchpad-reviews to: leonardr || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== salgado-lunch is now known as salgado
=== matsubara is now known as matsubara-lunch
jelmerHi leonardr18:45
jelmerCould I add a merge proposal to your queue?18:45
leonardrjelmer, sure18:46
jelmerleonardr: Thanks! The mp is https://code.edge.launchpad.net/~jelmer/launchpad/55288-publisher-unknown-distroseries/+merge/3223218:46
=== leonardr changed the topic of #launchpad-reviews to: leonardr || reviewing: jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
rockstarleonardr, us Tarmac guys wants yous to look at https://code.edge.launchpad.net/~jkakar/launchpadlib/fake-launchpad/+merge/26391 please.18:57
leonardrrockstar, ok18:57
=== leonardr changed the topic of #launchpad-reviews to: leonardr || reviewing: jelmer, rockstar || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
rockstarleonardr, I realize that it's a HUGE diff.18:58
rockstarleonardr, technically it's jkakar's branch, but I'm willing to help facilitate where needed.18:58
jelmerrockstar: Ooh, a fakelaunchpad, nice!18:59
marsleonardr, +1 from Ursinha, lifeless, Diogo and I on having that as well.  The QA tools use launchpadlib a lot18:59
leonardrrockstar: this branch has not had any code revisions since i made some sweeping comments back in may19:00
leonardri can take another look but a lot of it is going to boil down to "you should really make those changes"19:00
rockstarleonardr, okay, great.19:00
rockstarWhat's more, I think jkakar is on holiday for a bit.19:02
EdwinGrubbsjelmer: do you want to finish reviewing this feature? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-602385-register-project-from-sourcepackage-page-part2/+merge/3223419:08
EdwinGrubbsjelmer: oops, I saw your name at the top, and I thought you were oncall. I'll ask leonardr.19:08
EdwinGrubbsleonardr: can you review https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-602385-register-project-from-sourcepackage-page-part2/+merge/3223419:09
=== EdwinGrubbs changed the topic of #launchpad-reviews to: leonardr || reviewing: jelmer, rockstar || queue: [Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== leonardr changed the topic of #launchpad-reviews to: leonardr || reviewing: jelmer || queue: [Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== matsubara-lunch is now known as matsubara
salgadoleonardr, can I add myself to the queue with https://code.edge.launchpad.net/~salgado/lazr.restful/fix-exporting-empty-entries/+merge/32239 ?19:51
leonardrsalgado: yeah, but i think you're the limit19:51
=== salgado changed the topic of #launchpad-reviews to: leonardr || reviewing: jelmer || queue: [Edwin,salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
leonardrjelmer, r=me20:06
=== leonardr changed the topic of #launchpad-reviews to: leonardr || reviewing: Edwin || queue: [salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jelmerleonardr: Thanks!20:09
leonardredwin, some stupid questions20:15
leonardrwhat does the 'count' attribute do in the img tag within the copyright-expand link?20:15
leonardredwin: is source_package_name really a name? it looks like a source_package object that _has_ a name20:18
EdwinGrubbsleonardr: the count can be removed. I copied the link from elsewhere in the widget template, although I don't understand what it does for those links. I'll investigate and see if it is just cruft for those links also.20:23
leonardrok20:23
EdwinGrubbsleonardr: confusingly, the sourcepackage object is just a combination of a distroseries and a sourcepackagename object, and the sourcepackagename table in the db just has a name column.20:24
leonardrok, so it is a sourcepackagename object that has a name20:25
EdwinGrubbsleonardr: yes, the sourcepackage.name actually just returns the sourcepackagename.name20:26
leonardredwingrubbs, does link_source_package need to check that self.product is not None, or is that a given?20:29
EdwinGrubbsleonardr: hmm, it won't be None because of where it is called, but I should turn that into an argument to link_source_package() so that it does surprise someone when they reorganize the main_action().20:31
leonardrok20:32
bacleonardr:  do you have time for another branch to review?20:33
jelmerI was about to ask the same :-)20:33
bacjelmer:  trade?20:34
bacjelmer:  meaning, i'll review your branch if you'll do mine.20:35
leonardredwingrubbs: "If that is your situation we u= rge" really? is that an email thing?20:35
leonardrif so, it might make more sense to change it back, to make it clear where the line break is in the email20:35
EdwinGrubbsleonardr: oh, I was fixing a lint error. I'll try to make it look less odd without making lint complain.20:36
jelmerbac: Sure! What's your branch?20:38
bacjelmer:  https://code.edge.launchpad.net/~bac/launchpad/bug-590813/+merge/3224820:39
leonardredwin: r=me with the changes we discussed20:43
=== leonardr changed the topic of #launchpad-reviews to: leonardr || reviewing: salgado || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jelmerbac: Mine is at https://code.edge.launchpad.net/~jelmer/launchpad/publisher-use-debian-1/+merge/3224520:45
leonardrsalgado, your changes to test_webservice, are they just delinting?20:53
salgadoleonardr, yes, removal of unused imports20:54
leonardrok20:54
jelmerbac: Thanks for the review.20:54
leonardrsalgado, r=me20:54
salgadoleonardr, cool, thanks!20:55
bacjelmer:  np20:55
salgadoleonardr, no need to bump the version or add anything to NEWS.txt as there was no release where that bug was present, right?20:59
leonardrsalgado: right, i'll do a release tomorrow20:59
jelmerbac: I'm still reading up on snapshots, but a review of your branch is in progress as well.21:00
bacjelmer:  take your time, no rush21:00
=== leonardr changed the topic of #launchpad-reviews to: On call: - || reviewing: salgado || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== leonardr changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jelmerbac: It seems like this could be an issue in a lot of situations - every sql based collection field I guess?21:07
bacjelmer:  yes, that is true.21:07
jelmerbac: r=me, though I wonder if it would make sense to have a more structural solution for this kind of thing as well.21:08
jelmerbac: Something like a shortlist() implementation that would give up on trying to collect the full sequence contents rather than errorring out perhaps.21:09
jelmerIt seems like a pity to give up on snapshotting product series (which would be access quite often I imagine) just because of a very rare case.21:10
=== Ursinha is now known as Ursinha-lunch
bacjelmer:  my understanding of 'shortlist' is that we want it to complain loudly b/c it indicates our original assumptions were wrong21:14
bacjelmer:  lifeless started an email thread about changing Snapshot to ignore CollectionFields by default.  it seems like a good idea which may get implemented in the future.21:15
jelmerbac: Ah, that would make sense indeed.21:16
=== Ursinha-lunch is now known as Ursinha-afk
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
=== Ursinha-afk is now known as Ursinha

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!