/srv/irclogs.ubuntu.com/2010/11/23/#launchpad-reviews.txt

wallyworld_StevenK: bambi's b1tch asked me to flick a couple of reviews your way. you should see them in your email. thanks in advance :-)02:07
thumperwallyworld_: my headset isn't working03:18
wallyworld_thumper: ok, talk later then i guess :-)03:18
thumperI can hear you03:18
thumperbut mumble can't hear me :(03:18
henningejtv:Hi!07:56
henningejtv: I am surprised this works: http://paste.ubuntu.com/535469/07:56
jtvhi henninge!07:56
jtvQ about my MP?07:56
henningeyes07:56
jtvIt doesn't for me.07:56
jtvBut give my http connection a bit more time.07:57
jtvAhhh there it is.07:57
henningemakeSourcePackage makes an *Ubuntu* source package?07:57
jtvWhat's the surprise?07:57
=== StevenK changed the topic of #launchpad-reviews to: On call: - || Reviewing: || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
jtvHeh, I hadn't thought of that. But we now basically assume that all source packages are Ubuntu, don't we?07:58
StevenKWe shouldn't07:58
jtvBecause I think you're right: it creates a new distro AFAIK.07:58
jtvStevenK: this is Translations, it's special.  :)07:58
henningejtv: we do???07:59
henningeoh, right, we do .. we are Translations ....07:59
henninge;-)07:59
jtvWe are special.07:59
jtvOur new data model accommodates only one family of distributions.  Once we have the sharing working, we can worry about shoehorning in more—if required.08:00
henningeSo it's our checking if productseries is None -> it's ubuntu08:00
henningebut it wasn't in focus ;)08:00
jtv'zacly08:01
henningejtv: but still, I always use the "ubuntu" celebrity, I think.08:01
henningeto be explicit08:01
jtvThat's perfectly correct, just very verbose.  My personal feeling is we'll want a makeUbuntuSourcePackage at some point.08:02
jtvOr maybe even a makePOFile/makePOTemplate option for_ubuntu.08:02
henningeyes, our dear Ubuntu. :-)08:02
jtvOr in this case, our ubiquitous ubuntu.08:03
henningejtv: so can you please be verbose in this test and use ubuntu?08:03
jtvOur user-friendly but undeniably un-upstream ubiquitous ubuntu.08:03
henninge"un-upstream" as in "not is_current_upstream!08:04
henninge"08:04
henninge?08:04
jtvWhy?  We're assuming that it'll always be either Ubuntu or a derivative.  There's no easy way for the code to discover the difference.  If this ever becomes not what we want, we'll have more places to change and that would be a good time to come up with something more convenient.08:04
jtvExactly.08:07
henningeok, ok ...08:07
jtvIt's really über-un-upstream.08:07
henningejtv: template.getTranslationPolicy08:08
jtvuh-huh08:08
henningeI guess you added that in a recent branch?08:08
jtvYes.  It gets the ITranslationPolicy implementation that applies to the template.08:08
jtv(Which is either a Project or a Distribution, though a Project may also inherit part of its policy from a ProjectGroup)08:09
jtvI considered calling it getPillar, but we (including yours truly) have been overusing that a bit, and it's not quite right here.08:10
jtvThis also gives us the option to break out the repetitive policy data into its own ORM-backed class in the future.08:11
henningecool08:12
henningejtv: two more things08:13
jtv(I'm sitting here facing a lady reading a house-and-garden magazine.  The rear cover shows either a palace or a stately mansion with an elephant either grazing in the massive land around it, or carrying the house on its back.  The perspective makes it unclear.  The front cover says "Humble Living.")08:14
jtvShoot.08:14
henningeIt feels to me like there should be more test_revert_unselected_translations where plural_indices_to_store has a value other than []08:14
henningethere is only one so far08:15
henningeonly a feeling, though. I did not check test coverage very closely.08:15
henningejtv: also, I think it would be cool to implement contains_translations using "any" ;-)08:16
jtvI considered both, believe it or not.08:16
henningeany([text is not None and len(text) != 0 for text in translations.itervalues()])08:17
henningeok08:17
henningethen we are done, I guess08:17
* henninge knows that jtv always considers thoroughly.08:17
henningeoh, i thing the [...] are not even needed in 2.608:18
henninges/thing/think/08:18
jtv:-)  Thanks.  Two reasons I didn't use any(): call it micro-optimization but I didn't feel like constructing a list for something this small; and to make it look really _nice_ I'd have to evaluate text as a boolean, which we're not really supposed to do.08:19
jtvOh, without the [] we'd probably avoid the list…08:19
jtv…but that wasn't the main thing.  :)08:19
jtvAbout the tests: I could add test for various set relationships, but it seemed a bit redundant.  As they say, one needs to test the zero case and the one case, and all else just follows.08:20
henningeyes but I was just thinking of more conditions around [0]08:21
jtvOK, I'll spend a bit more time (not too much :) thinking about things I could test for.  It's a valid concern.08:21
jtvBut now I need to run to a place where I can't conveniently use wifi.08:22
henningealthough I had not heard "them" say that yet. Sounds sound, though.08:22
henningerun!08:22
jtvAssuming I want to keep this laptop dry.08:22
jtvWell, "hop" is more accurate at this stage.08:22
jtvLimp.08:22
jtvScurry.08:22
StevenKgmb: O hai, you're free to do some reviews?09:17
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbStevenK: I am now.10:03
=== gmb changed the topic of #launchpad-reviews to: On call: gmb || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
StevenKgmb: That SHA1 comes from sampledata10:27
StevenKIt actually comes from the gina_archive in the source tree10:28
gmbStevenK: So, can we update the source so that it's obviously manufactured?10:29
StevenKgmb: I can update the test to extract the same changelog, run sha1 over it, and then compare them if that would please you more?10:30
gmbStevenK: That would work, yes. Please do it.10:33
StevenKgmb: I could change it to do .read() and check the output, if you don't like the sha110:46
gmbStevenK: Even better.10:47
StevenKgmb: Changed pushed, diff updated10:59
=== matsubara-afk is now known as matsubara
adeuringgmb: could you please review this mp: https://code.edge.launchpad.net/~adeuring/launchpad/bug-675595/+merge/41595?13:39
gmbadeuring: Sure13:40
adeuringthanks!13:40
gmbadeuring: r=me. Nice work.13:50
adeuringgmb: thanks!13:50
=== matsubara is now known as matsubara-lunch
=== mars changed the topic of #launchpad-reviews to: On call: gmb,mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== mars changed the topic of #launchpad-reviews to: On call: gmb,mars || Reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbadeuring: I'm heading out for a run, so just put whatever you need reviewing on the queue and I'll attend to it when I return.14:17
gmbOh, or maybe mars will look first :). Hi mars.14:17
marsHi gmb14:17
adeuringgmb: ok, thanks for the notice14:17
adeuringgmb, mars: could one of you please have a look at https://code.edge.launchpad.net/~adeuring/launchpad/bug-675595-2/+merge/41604 ?14:46
marsadeuring, sure14:49
adeuringmars: thanks!14:49
=== mars changed the topic of #launchpad-reviews to: On call: gmb,mars || Reviewing: -,adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== mars changed the topic of #launchpad-reviews to: On call: gmb,mars || Reviewing: -,adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
sinzuisalgado can you review https://code.launchpad.net/~sinzui/launchpad/upstream-downstream-bug-links-0/+merge/4137114:58
salgadosinzui, sure, but I may not be able to get to it today.  can it wait until tomorrow?14:58
sinzuiokay14:58
salgadocool14:59
=== salgado is now known as salgado-lunch
=== matsubara-lunch is now known as matsubara
=== mars changed the topic of #launchpad-reviews to: On call: gmb,mars || Reviewing: -,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
adeuringmars: thanks for the review!15:23
=== sinzui changed the topic of #launchpad-reviews to: On call: gmb,mars || Reviewing: -,- || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
sinzuigmb, mars: I have a small branch that needs review: https://code.launchpad.net/~sinzui/launchpad/private-persontransferjob-0/+merge/4161415:54
gmbsinzui: I'll take it.15:54
=== gmb changed the topic of #launchpad-reviews to: On call: gmb,mars || Reviewing: sinzui,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
gmbsinzui: Please add a comment / docstring to test_no_warning_for_PersonTransferJob() explaining the expected behaviour. Otherwise, r=me.15:57
sinzuigmb: thanks. I will update the test15:58
gmbCool.15:59
=== gmb changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
* gmb calls it a day for reviewing15:59
=== deryck is now known as deryck[lunch]
abentleymars: could you please review https://code.edge.launchpad.net/~abentley/launchpad/more-harness/+merge/41619 ?  It's tiny.16:23
marsabentley, about to go on a call, but I will as soon as possible afterwards16:29
abentleymars: also: https://code.edge.launchpad.net/~abentley/bzr-builder/launchpad/+merge/4162317:09
=== abentley changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [abentley, abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== salgado-lunch is now known as salgado
=== deryck[lunch] is now known as deryck
=== benji is now known as benji-lunch
=== maxb_ is now known as maxb
=== benji-lunch is now known as benji
=== mars changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-afk
=== salgado is now known as salgado-afk
=== sinzui changed the topic of #launchpad-reviews to: On call: mars || Reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
sinzuimars, do you have time for a short branch? https://code.launchpad.net/~sinzui/launchpad/prefill-homepageurl-0/+merge/4165221:00
marssinzui, sure21:14
=== mars 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
sinzuithanks21:14
thumperabentley: https://code.edge.launchpad.net/~thumper/launchpad/recipe-binary-builds/+merge/4068622:04

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