=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: - || queue: [noodles775,wgrant,wgrant, noodles775, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[09:22] <noodles775> Hi intellectronica! The first one in the queue there for me is: https://code.edge.launchpad.net/~michael.nelson/launchpad/567922-binarypackagebuild-new-table-5/+merge/25239 when you've time.
[09:23] <noodles775> (just see the note about conflicts with db-devel on the MP... you can branch it rather than merge to run the tests).
[09:23] <intellectronica> noodles775: yes, looking at it already
[09:23] <noodles775> intellectronica: great, thanks.
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: noodles775 || queue: [wgrant,wgrant, noodles775, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[09:53] <intellectronica> noodles775: r=me
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: - || queue: [wgrant,wgrant, noodles775, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[09:54] <noodles775> Thanks intellectronica
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: wgrant || queue: [wgrant, noodles775, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: wgrant || queue: [noodles775, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[10:08] <jelmer_> hi intellectronica
[10:08] <jelmer_> intellectronica: Can I add another branch to the queue?
[10:08] <intellectronica> jelmer_: sure
=== jelmer_ changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: wgrant || queue: [noodles775, sinzui, sinzui, jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[10:09] <jelmer_> thanks :-)
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: sinzui || queue: [noodles775, sinzui, jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: - || queue: [noodles775, sinzui, jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[11:08] <intellectronica> noodles775: so the next in line is 567922-binarypackagebuild-new-table-6 ?
[11:08] <noodles775> intellectronica: indeed, thanks.
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: noodles775 || queue: [jelmer] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[11:23] <adiroiban> henninge: hi. was this branch landed https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525992/+merge/23285 ?
[11:24] <henninge> adiroiban: no, don't think so. That was probably due to ec2 problems we were (are?) having.
[11:25] <henninge> adiroiban: can you merge the current devel, please, before I try again?
[11:25] <adiroiban> henninge: yep. I will let you know when the merge was pushed.
[11:26] <henninge> thanks
[11:56] <adiroiban> henninge: merge pushed. no conflicts.
[11:57] <henninge> adiroiban: cool, I'll land it
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: jelmer || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[12:04] <intellectronica> gimme some more soyuz branches. ---- yeah!
[12:07] <jelmer_> :-)
[12:07] <jelmer_> it's more registry than soyuz I think
[12:09] <intellectronica> yes. looks fine too.
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== adiroiban changed the topic of #launchpad-reviews to: On call: intellectronica || reviewing: - || queue: [adiroiban(bug-525371)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[12:14] <adiroiban> danilos, henninge, jtv : I have created a MP for the first step in having a POTemplate API. Maybe you would like to take a look https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525371/+merge/25423 :)
[12:15] <jtv> adiroiban: I can take that if you like
[12:15] <adiroiban> jtv: feel free to to it :)
[12:16] <henninge> jtv: Adi and I had a pre-imp discussion about this at UDS.
[12:38] <adiroiban> adeuring: hi. Is there anything I need to do for having this branch approved and landed https://code.edge.launchpad.net/~adiroiban/launchpad/bug-402235/+merge/23973 ?
[12:40] <adeuring> adiroiban: nothing than reminding me. Sorry, I tried to get the branch in for the last release, but the ec3 test hung unfortunately. And I was on holidays last week, was conpletel offline for a few days. I'll stat the ec2 test now again.
[12:41] <adiroiban> adeuring: don't worry :) it is not a critical bug.
=== barry` is now known as barry_
=== mrevell is now known as mrevell-lunch
=== jelmer_ is now known as Guest6486
=== mrevell-lunch is now known as mrevell
=== abentley changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: -,- || queue: [adiroiban(bug-525371)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:08] <adiroiban> danilos: are there any plans for allowing project maitainers to add custom language codes? this can help with fixing bug 487137
[14:08] <mup> Bug #487137: Allow Rosetta admins to create custom language codes <Launchpad Translations:In Progress by adiroiban> <https://launchpad.net/bugs/487137>
[14:10] <danilos> adiroiban, not at this time, no... I am not sure that's a good idea yet, but it's what we should aim for in the future (exposing people to more and more unfinished features would make LP only harder to use)
[14:10] <adiroiban> danilos: ok
=== Ursinha_ is now known as Ursinha
[14:39] <cody-somerville> Anyone interested in doing a review? :-)
[14:39] <cody-somerville> https://code.edge.launchpad.net/~cody-somerville/launchpad/create-ppa-via-api/+merge/25319
[14:53] <cody-somerville> abentley, intellectronica: ^^ :-)
[14:53] <intellectronica> cody-somerville: otp, but can look at it after that
[14:54] <cody-somerville> kk
[14:56] <danilos> bigjools, this is logging I want to add to our jobs on the build slave, any comments? https://code.edge.launchpad.net/~danilo/launchpad/bug-580345/+merge/25416
[15:00] <danilos> jtv, henninge: ^^
[15:00] <intellectronica> cody-somerville: oright, the ppa api branch?
[15:01] <cody-somerville> create-ppa-via-api, yup
[15:01] <intellectronica> cody-somerville: is it really ok to create a ppa with just a name?
[15:02] <intellectronica> i mean, is it the same in the web ui?
[15:02] <bigjools> no it's not
[15:02] <cody-somerville> displayname looks optional
[15:02] <intellectronica> cody-somerville: there you go
=== Guest6486 is now known as jelmer_____
[15:02] <intellectronica> looks?
[15:02] <henninge> bigjools: huh?
[15:03] <bigjools> henninge: huh?
[15:03] <henninge> bigjools: what question are you replying to?
[15:03] <henninge> danilos: looks good to me.
[15:03]  * bigjools sees danilos' message and looks
[15:04] <cody-somerville> bigjools, are you says its not okay to create a PPA with just a name?
[15:04] <cody-somerville> *saying
[15:05] <bigjools> cody-somerville: ideally no
[15:05] <intellectronica> cody-somerville: i would rename the ubuntu variable to distribution. it's only a cosmetic change, but it means that if you ever want to start handling other distros it will be more natural. you can put the xxx comment near where you assign ubuntu to the distribution variable
[15:05] <bigjools> otherwise the +ppas page looks crap
[15:05] <cody-somerville> bigjools, It doesn't use the name if displayname is not set?
[15:06] <intellectronica> cody-somerville: i'm a bit concerned that this will be open to misuse. how expensive is it to create new ppas? should we maybe export the method for removing them, so that if someone creates too many they can delete them easily?
[15:07] <intellectronica> bigjools: consider this too ^^^
[15:07] <cody-somerville> There is no cost to creating a new PPA.
[15:07] <cody-somerville> and yes, I intend to export the ability to delete as well.
[15:07] <bigjools> there's always a cost :)  intellectronica, I said that this was ok since we added deletion now
[15:07] <intellectronica> cody-somerville: i'd feel more comfortable if you said that the cost is very low :)
[15:08] <intellectronica> bigjools: you mean there's already an api for deleting them, or that you can delete them using the web interface?
[15:08] <bigjools> danilos: I just added a comment to your MP
[15:08] <bigjools> intellectronica: just the web interface ATM, but as Cody said he's exporting that too
[15:09] <intellectronica> cool
[15:09] <cody-somerville> intellectronica, Sorry, there wasn't enough significant digits in my calculation to provide that level of precision :P
[15:09] <intellectronica> is there a conclusion about whether displayname and description are really optional?
[15:10] <intellectronica> cody-somerville, bigjools: ^^^^^
[15:10] <intellectronica> other than that the branch is fine
[15:10]  * bigjools looks at branch
[15:10] <danilos> bigjools, nooooo, anything but a comment!
[15:11] <danilos> bigjools, it's there because I played with different approaches to logging (ideally I'd use Launchpad logger, but didn't want to think about how I get that installed onto build slaves as well)
[15:12] <cody-somerville> intellectronica, displayname in not optional in web UI but description is
[15:13] <bigjools> cody-somerville. intellectronica: there's some refactoring needed
[15:13] <bigjools> the code is slightly duplicating the save_action() in the browser code
[15:14]  * cody-somerville nods.
[15:14] <cody-somerville> I was thinking the same thing
[15:14] <bigjools> cody-somerville: and in fact it misses a bunch of validation
[15:14] <intellectronica> cody-somerville: if you can make the web code use the same model code for doing that it would be good
[15:15] <bigjools> see also validate() in the browser code
[15:15] <intellectronica> cody-somerville, bigjools: i think i'll leave you to it, then ;)
[15:15] <bigjools> I'll comment on the MP
[15:15] <cody-somerville> bigjools, I did look at validate.
[15:16] <cody-somerville> bigjools, a lot of it isn't necessary
[15:16] <bigjools> cody-somerville: why?
[15:16] <cody-somerville> bigjools, because that view is a hack
[15:16] <cody-somerville> bigjools, its different depending if you're creating your first PPA or if you're creating a second PPA.
[15:17] <bigjools> cody-somerville: the validation on the form takes that into account
[15:17] <bigjools> but your new code does not
[15:17] <bigjools> it also doesn't check for duplicate names
[15:18] <cody-somerville> bigjools, 1. and isn't necessary since you're always supplying the name whereas the form does not
[15:18] <cody-somerville> 2. It'll error out if you try that which seems fine to me since it can't provide any more meaningful feedback via the webservices API to my understanding.
[15:18] <bigjools> cody-somerville: that doesn't matter, you still need to check that it's the first PPA and then check if the CoC is signed
[15:19] <cody-somerville> bigjools, Thats what the discussion was about with jml on Friday night.
[15:19] <bigjools> re. (2), you're not testing that so you don't know by definition ;)
[15:19] <bigjools> yes, we said we'd think about ToS-ing the Coc or something
[15:20] <cody-somerville> also, the browser class doesn't check if the CoC is signed either
[15:20] <cody-somerville> and we can't check if the PPA ToS is signed since its not actually recorded
[15:20] <bigjools> cody-somerville: it checks to see if you have accepted the ToS
[15:21] <cody-somerville> bigjools, using a hack. Its not actually recorded.
[15:21] <cody-somerville> bigjools, which is why I added a note in the function docstring that using the API constitutes accepting the PPA ToS.
[15:21] <bigjools> hack?
[15:21] <bigjools> ok that's fine I think
[15:22] <cody-somerville> bigjools, Yes. It doesn't record it. If a PPA exists, it assume the ToS is accepted. If no PPAs exist, then it'll try creating the default PPA and provide that different view that has that checkbox.
[15:22] <bigjools> but, I think you should add more tests where it adds the same name twice
[15:22] <bigjools> I suspect it'll generate an OOPS
[15:22] <bigjools> and we don't want that
[15:22] <cody-somerville> bigjools, I think thats fair feedback.
[15:23] <bigjools> I'll write on the MP
[15:23] <cody-somerville> bigjools, ty
[15:24] <cody-somerville> :-)
[15:24] <bigjools> cody-somerville: np, thanks for the change
[15:24] <bigjools> how was the hangover on Saturday? :)
[15:25] <cody-somerville> lol. didn't drink enough for one (un)fortunately.
[15:27] <cody-somerville> bigjools, how about yourself?
[15:27] <bigjools> cody-somerville: you were pretty hammered on Friday :)
[15:27] <bigjools> I was tired, slept like a baby on the train hime
[15:27] <bigjools> home
[15:29] <BjornT> cody-somerville, intellectronica, bigjools: don't forget to add tests for making sure who has permission to call the createPPA() method.
[15:30] <cody-somerville> Shouldn't there already be tests for that?
[15:30] <bigjools> cody-somerville: since you added the method, no :)
[15:30] <bigjools> you need a unit test for it
[15:30] <BjornT> cody-somerville: yes, the one who added the method should have added tests for that ;)
[15:31] <cody-somerville> createPPA is just a wrapper around new on ArchiveSet
[15:31] <bigjools> still needs a test dude
[15:32] <bigjools> you should use launchpadlib in the story test too
[15:32] <bigjools> I updated the MP
[15:33] <BjornT> cody-somerville: also think about the documentation. now you're saying that anyone can call createPPA. they shouldn't have to try to call it to find it out.
[15:35] <cody-somerville> very well.
[15:38] <bigjools> so, Ubuntu Software Centre fails under Kubuntu :/
=== deryck is now known as deryck[lunch]
=== barry_ is now known as barry
[16:16] <EdwinGrubbs> BjornT, have you had a chance to look at the schema changes in https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-535430-needspackaging-timeout-part1/+merge/25359
[16:23] <gmb> intellectronica, abentley: I have another branch for your queue if that's okay: https://code.edge.launchpad.net/~gmb/launchpad/show-oops-ids-bug-564686/+merge/25442
[16:24] <abentley> gmb, I'll take it.
[16:24] <gmb> abentley, Thank you.
[16:24] <BjornT> EdwinGrubbs: no, i haven't. i'll look now
=== adiroiban changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: -,- || queue: [adiroiban(bug-570899), adiroiban(bug-525371)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== abentley changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: -,gmb || queue: [adiroiban(bug-570899), adiroiban(bug-525371)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:31] <abentley> gmb, is it possible to use the same code path for generating oops URLs that other code does?
[16:33] <gmb> abentley, I looked into that, and tried to reuse the code in webapp/tales.py, but there didn't seem to be a simple way to do use it (I thought there would be a formatter, but apparently not). I'll have a dig around, though, see if I can find anything else.
=== intellectronica changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: adiroiban(bug-570899),gmb || queue: [adiroiban(bug-525371)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:35] <intellectronica> adiroiban: you've got several jslint notices. what about them?
[16:37] <intellectronica> adiroiban: also, why can't you test?
=== matsubara is now known as matsubara-lunch
[16:41] <gmb> abentley, I can't find anything reusable so far; will keep looking but I think we'd be better off having a formatter do the work for us. I might as well fix that in this branch.
[16:41] <abentley> gmb, that would be great if you don't mind.
[16:41] <gmb> abentley, Sure, no problem.
[16:57] <BjornT> EdwinGrubbs: remind me, what is section, and can it change between different series?
[16:58] <EdwinGrubbs> BjornT, section for debs are "apps", "graphics", etc.
=== deryck[lunch] is now known as deryck
[17:01] <BjornT> EdwinGrubbs: ok. let's say a package was in "apps" in karmic, and "graphics" in lucid. first a new release is uploaded to lucid, and then one is uploaded to karmic. what value will DistributionSourcePackage.section have?
[17:03] <BjornT> EdwinGrubbs: also, what happens if a PPA package is uploaded?
[17:04] <EdwinGrubbs> BjornT, that's an interesting point. The trigger which caches it, gets the latest one. I'm not sure what the current behavior is for the page that gets the data directly from the SPPH table.
[17:04] <BjornT> EdwinGrubbs: so, i would suggest discussing this patch with bigjools
=== gary_poster is now known as gary-lunch
[17:05] <EdwinGrubbs> BjornT, I forgot, I need to select only PRIMARY packages to eliminate PPAs.
[17:06] <bigjools> EdwinGrubbs: why are you using a trigger?
[17:10] <bigjools> EdwinGrubbs: BjornT also has a good point about which series' data you're caching
[17:10] <EdwinGrubbs> bigjools, since we would like to consolidate the DistributionSourcePackage class and the DistributionSourcePackageInDatabase class, the object will need to exist in the db by the time the code will use it. This could be done with a cronjob, but that actually seems like it would have more intensive queries, since it would have to query all the SPPHs for a distr+spn in order to find the one with the latest date.
[17:11] <bigjools> I suspect it should only be the one that's marked 'current' for each distro
[17:14] <bigjools> Since DSP is going to start getting published-specific data, you need to define which series that will be for, and how it gets updated when a new series is opened
[17:15] <bigjools> I think you can do that without a trigger or cron job
[17:15] <EdwinGrubbs> bigjools, where is a distroseries specified as current?
[17:16] <bigjools> its status
[17:18] <EdwinGrubbs> bigjools, when you say that I can do that without a trigger or cronjob, do you want the class to automatically create the DSP record whenever it is requested?
[17:19] <bigjools> EdwinGrubbs: I think that's a good idea, yes.  It would only happen at package upload time.
[17:19] <bigjools> (which is never a webapp transaction)
[17:20] <bigjools> so when creating a publishing record, you can check to see if it's the right series and then update/create the DSP
[17:20] <bigjools> this will also be good for Soyuz when we start caching stuff :)
[17:21] <EdwinGrubbs> bigjools, which script creates the spph record? I assume it's triggered by dput.
[17:21] <bigjools> and it's also an easy check to see if it's the PRIMARY archive
[17:21] <bigjools> EdwinGrubbs: let me check where the best place to do this is
[17:23] <adiroiban> intellectronica: back. I don't know how to fix those jslint.
[17:24] <bigjools> EdwinGrubbs: see lib/lp/soyuz/model/publishing.py
[17:25] <bigjools> EdwinGrubbs: IPublishingSet.newSourcePublication()
[17:25] <adiroiban> intellectronica: as for LEFT and RIGHT keys, it looks like Windmill does not support trigering such events
[17:26] <bigjools> EdwinGrubbs: you need to check that the archive.purpose = PRIMARY, then you can see if the DSP exists or not and create/update
[17:26] <bigjools> that's much easier to test than a trigger :)
[17:26] <adiroiban> intellectronica: here is the windmill bug http://github.com/windmill/windmill/issues#issue/33
[17:27] <bigjools> EdwinGrubbs: have you figured out which series you need to cache data for?
=== abentley changed the topic of #launchpad-reviews to: On call: intellectronica, abentley || reviewing: adiroiban(bug-570899), - || queue: [adiroiban(bug-525371)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[17:27] <bigjools> bearing in mind development is newer than current
=== abentley is now known as abentley-lunch
[17:28] <bigjools> having said that, section is not likely to change
[17:31] <intellectronica> adiroiban: ok, let me see if i can get rid of the jslint errors. we can't land with them.
[17:35] <adiroiban> intellectronica: I had those errors in other branches and Danilo said that it is ok and this is the way YUI3 works
=== matsubara-lunch is now known as matsubara
[17:37] <intellectronica> adiroiban: i rather check first, and at least register the problem somehow. it's not a good idea to keep jslint errors in the codebase.
[17:37] <danilos> adiroiban, I don't remember saying so unless if it was a branch where you didn't touch the faulty JS which I don't want to hold you responsible for :)
[17:41] <EdwinGrubbs> bigjools, I don't know for which series the section should be cached. Maybe sinzui knows.
[17:42] <intellectronica> adiroiban: also, i think that at this point it would make sense to assign the magic numbers to CONSTANTS, but I won't ask you to do it because i see that you're just extending legacy code.
[17:42] <sinzui> series cache>
[17:42] <bigjools> EdwinGrubbs: it's an important question since this table will be useful beyond your use case I think
[17:42] <sinzui> Is someone watching my keystrokes as I addtal:content="cache:public, 1 hour"
[17:42] <sinzui> to series templates?
[17:42] <bigjools> \m/
[17:43] <adiroiban> intellectronica: I will create those contants as it should improve the code readability
[17:43] <intellectronica> adiroiban: excellent, thanks!
[17:44] <sinzui> bigjools, I think I am working on the issue you are talking about. I am working on the timeouts for distroseries and milestones that dominate oops
[17:45] <bigjools> sinzui: I am checking over Edwin's branch, I guess you're both working on this?
[17:46] <gmb> abentley-lunch, I've added a formatter for oops IDs and some tests to go with it. I've pasted a diff in the MP for your perusal.
[17:46] <sinzui> bigjools, I made the initial proposal. and I liked EdwinGrubbs suggested changes. I have not looked at the branch though
[17:48] <sinzui> bigjools, I suppose you are remarking that once a series is released, the packages should not change.
[17:48] <sinzui> is backports main?
[17:49] <intellectronica> adiroiban: that code is a bit weird, no wonder jslint complains
[17:49] <bigjools> sinzui: well a couple of things, 1. we should do it in code not in a trigger (soyuz will benefit from cache publishing data in places so I want to prepare for that), 2. yes a released series won't change
[17:50] <intellectronica> danilos: ^^^^^
[17:50] <bigjools> sinzui: backports is a pocket, not sure what you mean?
[17:50] <bigjools> you mean the primary archive?
[17:50] <sinzui> bigjools, thanks for your incite
[17:50] <sinzui> yes I mean primary archive
[17:51] <bigjools> so yes -backports is the primary archive
[17:51] <sinzui> yuck
[17:51] <bigjools> problems?
[17:52] <sinzui> um, no actually. I had a momentary lapse of reason
[17:52] <bigjools> I get that all the time :)
[17:52]  * sinzui is still thinking of series and milestones
[17:55] <EdwinGrubbs> sinzui, on which page will you be querying the cached section from the DSP table? It doesn't look like that is used on the +needs-packaging page.
[17:55] <adiroiban> intellectronica: let me rewrite it. what do you consider to be weird ? :)
[17:57] <intellectronica> adiroiban: i don't understand what is them `stem` second argument that the even handlers receive
[17:58] <intellectronica> adiroiban: oh, is it the last argument you pass to on()?
[17:58] <sinzui> EdwinGrubbs, I think the root page is the only user of memcached. I am wrapping portlets.
[17:58] <sinzui> EdwinGrubbs, I have not tested memcached with a batched listing. I do not think it will work
[18:05] <adiroiban> intellectronica: let me rewrite that code as I realise it is just stupid :)
[18:05] <intellectronica> adiroiban: i have a solution for the first batch of lint errors, but it ain't pretty :)
[18:06] <intellectronica> adiroiban: what were you thinking of doing?
[18:06] <adiroiban> intellectronica: those functions are adapters for toggleWidget()
[18:07] <adiroiban> as the YUI3 will pass the event as the first argument
[18:07] <adiroiban> I can not use them directly with Y.on
=== gary-lunch is now known as gary_poster
[18:09] <intellectronica> adiroiban: yeah, got it. the solution i have is to enclose the handlers in a closure. it isn't necessary in this case, since you're not capturing anything but using yui's event machinery to pass an argument, but jslint is correct to make that suggestion, and a nicer solution would require refactoring the code.
[18:09] <EdwinGrubbs> sinzui, What does memcached have to do with the DistributionSourcePackage.section column?
[18:09] <intellectronica> adiroiban: a nicer solution might be to move the body of the loop into a separate function
[18:10] <sinzui> EdwinGrubbs, memcached+tales caches page fragments. Which page/part do you want me to test?
[18:11] <adiroiban> intellectronica: i was thinking to create those adapter functions outside the loop
[18:12] <intellectronica> adiroiban: yes, that's also a possibility. whichever you prefer.
[18:21] <sinzui> EdwinGrubbs, memcached+tales does include the query string in the key. I think we can cache batches
[18:21] <EdwinGrubbs> sinzui, I'm adding the section column to the DistributionSourcePackage table to cache the data from the SPPH table. I think you told me to do that on Friday. I don't know where the query is that we are trying to speed up. This should be unrelated to memcached.
[18:23] <sinzui> EdwinGrubbs, yes, it is unrelated. To exclude language-packs, we use the section which is know in spph or in spr. I used spph in the branch I am landing today
[18:23] <sinzui> EdwinGrubbs, yes, it is unrelated. To exclude language-packs, we use the section which is know in spph or in spr. I used spph in the branch I am landing today
=== intellectronica changed the topic of #launchpad-reviews to: On call: abentley || reviewing:  - || queue: [adiroiban(bug-525371)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== EdwinGrubbs_ is now known as foo1000
[19:14] <leonardr> abentley, i'd like to add https://code.edge.launchpad.net/~leonardr/lazr.restful/understand-mod-compress/+merge/25454 to the review queue
[19:59] <mars> gary_poster, ping, have a moment to review https://code.edge.launchpad.net/~mars/launchpad/disable-windmill-tests/+merge/25457 ?
[19:59] <gary_poster> mars, ok
[20:03] <gary_poster> mars, you added in TESTOPTS for theoretical correctness, yes?
[20:03] <gary_poster> that is, no one is actually using them, it seems?
[20:03] <mars> gary_poster, it was already in there, just not used properly.
[20:04] <mars> they are used by three targets lower down in the file
[20:04] <gary_poster> ah, gotcha mars
[20:04] <mars> I was going to rename VERBOSITY to TESTOPTS myself, and was surprised to find TESTOPTS was already in there
[20:05] <gary_poster> yeah, another approach would be to rip it out, but I suppose this is safer
[20:05] <mars> using the value of the VERBOSITY variable to pass "-vvv -t --layer" to the test process really rubbed me the wrong way.  Refactoring gene I guess :/
[20:05] <mars> gary_poster, safer, yes.  I don't know who else used VERBOSITY that way.
[20:06] <gary_poster> mars, have you checked to see what happens to the test runner if you say --layer='!WindmillLayer' --layer='WindmillLayer'?
[20:06] <mars> Needed a fast fix.  This is one step forward.
[20:06] <mars> My guess is that --layer=!Layer --layer=Layer *will* execute the desired test suite.
[20:06] <gary_poster> well, for a fast fix, maybe leaving TESTOPTS out of the picture would be wiser.  Otherwise I have questions about it
[20:06] <mars> But no, I have not explicitly tested that :)
[20:07] <mars> gary_poster, oh?  What questions?
[20:07] <gary_poster> for instance--if you are right (which would be my guess) then I would suggest that ``bin/test $(VERBOSITY) $(TESTOPTS) --layer=WindmillLayer`` should instead be ``bin/test $(VERBOSITY) --layer=WindmillLayer $(TESTOPTS)`` so that TESTOPTS can actually override things
[20:07] <mars> gary_poster, I treat TESTOPTS like CFLAGS.  VERBOSITY is independent of the two.
[20:08] <mars> gary_poster, ok, interesting.  That woudl have come up regardless if I used VERBOSITY or TESTOPTS.
[20:09] <gary_poster> yeah, calling it TESTOPTS is more correct, and makes the possible problem clearer.
[20:09] <gary_poster> mars, I'll approve, simply asking that you consider my question...possibly later. :-)
[20:09] <mars> +1
[20:10] <mars> gary_poster, I'll send a post to the list when this lands.
[20:10] <gary_poster> mars, ok.  approved.
[20:10] <mars> a post about the suite disablement
[20:10] <gary_poster> right
[20:10] <mars> thanks gary_poster
[20:11] <gary_poster> mars, also note that when this branch lands the linked bug will be marked fix committed I think.  If so, I'm guessing (?) you'll want to manually change it back, since this is a bandaid not a fix.
[20:12] <mars> gary_poster, yep!  Had to do that a few times now.  The QA scripts also do that.
[20:12] <mars> I'm working against the grain!
[20:12] <gary_poster> :-)
=== abentley-lunch is now known as abentley
=== matsubara is now known as matsubara-afk
[21:16] <leonardr> abentley, i don't know if you're still reviewing but i've also got a launchpad follow-up to my earlier lazr.restful branch
[21:16] <leonardr> https://code.edge.launchpad.net/~leonardr/launchpad/understand-mod-compress
=== abentley changed the topic of #launchpad-reviews to:  On call: abentley || reviewing: leonardr || queue: [adiroiban(bug-525371)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[21:19] <abentley> leonardr, did you mean to target your branch at db-devel, rather than devel?
[21:20] <leonardr> abentley: i don't think so, i generally target devel--is devel closed atm or something?
[21:21] <abentley> leonardr, devel is not closed.  You have proposed for landing against db-devel.  This means that some other changes from devel are showing in your diff.
[21:21] <leonardr> abentley: i misunderstood your question
[21:21] <leonardr> i thought you were saying "you targeted your branch at devel, but you probably meant db-devel" rather than vice versa
[21:22] <abentley> leonardr, ah, thought so.
[21:22] <leonardr> abentley, i'm redoing the proposal
[21:22] <abentley> leonardr, thanks.
[21:23] <leonardr> done
[21:39] <abentley> leonardr, there should be two blank lines before a new header in a docstring.
[21:41] <abentley> leonardr, also, the differences between '"foo-gzip"' and '"foo"' info '"foo"-gzip' are pretty subtle.  It would be nice to mention them both at the same time.
[21:42] <abentley> e.g.:  mod_compress may turn '"foo"' into '"foo"-gzip' or '"foo-gzip"'.  We treat both forms as the same.
[21:43] <abentley> also, you say "info" where you mean "into".
=== abentley 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
[22:03] <leonardr> gary: abentley reviewed my launchpad branch but not the lazr.restful branch it uses. would you take a look?
[22:03] <leonardr> https://code.edge.launchpad.net/~leonardr/lazr.restful/understand-mod-compress/+merge/25454
[22:03] <gary_poster> leonardr: ack, looking
[22:13] <adiroiban> intellectronica: i have updated the JS code adding constanst for key codes and fixing jslint warnings.
[22:13] <gary_poster> leonardr: approved, no comments.  thank you
[22:13] <leonardr> gary, great