#launchpad-reviews 2010-07-01 <lifeless> sinzui: if you're back, I've moved the profile module to lp.services as per your request, but it looks to me like it does not meet the namespace policy about deps. I've mailed the bug <lifeless> s/bug/mp/ * rinze changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <rinze> sinzui, hi <noodles775> Hi salgado, if you've got a spare few mins, can you take a look at the issue I've identified at: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-get-or-create-from-identity/+merge/28885 <noodles775> Let me know if I'm not making sense on the MP. <salgado> noodles775, sure, I have it in my inbox and will get to it shortly <noodles775> salgado: thanks. <salgado> noodles775, in which case is the sreg extension not included in the response? <noodles775> salgado: normal login with existing account. ie. 1) start dev server, 2) click login 3) enter johncrosby@example.net:test 4) click login. In this case calling _getEmailAddressAndFullName raises the assert. <noodles775> It's not seen in the current code as _getEmailAddressAndFullName is only called if the account doesn't exist. <salgado> noodles775, that's weird because the code that generates the positive openid response is including the sreg extension unconditionally * salgado tries to dupe <salgado> indeed, duped <noodles775> Yeah, I'd assumed the sreg response should always be there, and was surprised too. <salgado> noodles775, but this is a bug in the testopenid implementation <noodles775> Where should I look? <salgado> testopenid == the OpenID provider we use when developing/testing <salgado> I'm checking it now <salgado> noodles775, lib/lp/testopenid/browser/server.py <noodles775> OK, so we can be confident that the staging/production SSO always include the sreg. <salgado> createPositiveResponse() adds the sreg extension, but it's empty <noodles775> Thanks. <salgado> noodles775, I think I've found the problem <salgado> noodles775, OpenIDLogin.render() asks for 'email' and 'fullname' in the sreg response <salgado> bug testopenid ignores that and only includes the nickname <salgado> s/bug/but <noodles775> Right... I see. <noodles775> So should it instead include *only* the requested items (email/fullname) or in addition to the nickname? <salgado> I think it's fine to include the 3 of them, but the nickname shouldn't be used anywhere <noodles775> OK, I'll update it... thanks salgado ! <noodles775> salgado: also, I assume you don't have time to review the actual branch, but if you could have a brief look and make sure you think it's a sane change, that would be great. <salgado> noodles775, sure, I can do that <noodles775> Ta. <salgado> noodles775, out of curiosity, why somebody needs a launchpad account to buy stuff from the software center? wouldn't a Ubuntu SSO account be more appropriate in this case? <noodles775> salgado: yes, the must already have an SSO account to make the payment, it's after they've paid for the software, we need to give them access to a private PPA (ie. a private ppa subscription), and that's only possible if you've got an LP account. <salgado> I see <salgado> and it'd be lots of work to make it possible for someone to have a private ppa subscription without having a Launchpad profile, I assume? <noodles775> Yes, but that is the long-term plan. <salgado> cool! <salgado> 415 + except LookupError: <salgado> 416 + assert email_address is not None and full_name is not None, ( <salgado> 417 + "An email address and full name are required to " <salgado> 418 + "create an account.") <salgado> noodles775, I'd move that assert out of the except block <noodles775> That won't be required... I'd only added it because of the above issue. <noodles775> Hrm, although, it's still possible for other callsites in the future to pass in None (even if processPositive.. doesn't). <noodles775> So yep, will do. <salgado> indeed <salgado> noodles775, I think you can get rid of checked_for_email and the two email_set.getByEmail(email_address) calls by doing "email = email_set.getByEmail(email_address)" at the beginning instead of "email = None" <noodles775> salgado: yep - that was part of r9514 which I've reverted after our discussion (I couldn't do it at the beginning at that point, because email was None when called via processPositive..) <salgado> right <salgado> I guess I should reload the m-p page, then * noodles775 hasn't pushed... hangon. <noodles775> OK, pushed r9515... <salgado> noodles775, IPersonSet.getOrCreateByOpenIDIdentifier() has a requestor argument but the actual implementation doesn't <noodles775> salgado: thanks, I'll remove it from the IFace. I've just pushed 9516 which just adds the email/fullname to the sreg response for the testopenid server too. <salgado> cool, let me reload again <noodles775> Thanks salgado... I'll paste the irc conversation on the MP too. (And I've pushed the changes you suggested so far). I think that should be enough to get a normal review. <salgado> noodles775, np * noodles775 changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [sinzui, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <noodles775> jelmer-lunch: if you've time when you get back: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-get-or-create-from-identity/+merge/28885 <jtv> jelmer-lunch: my windmill tests pass now... I think there was some change in setup that we both needed to synchronize to. <jelmer_> noodles775: ok <jelmer_> jtv: I'll have another look, thanks. <noodles775> Sure. <jtv> noodles775: anything to add to jelmer's review? <noodles775> jtv: I pasted our irc conv. on the mp. <jtv> noodles775: that's all about us being rusty and making dirty jokes afaics, nothing about the branch itself... <jtv> Ah, apart from the bit about henning's xxx that triggered the jokes <jtv> rinze: the tests on my JS branch are passing now, repeatedly and consistently... do I get your Approval vote? <jtv> noodles775: if you approve of the js, maybe you can hit the js review button? <noodles775> jtv: no, I was just making sure rinze hadn't missed anything obvious... I didn't do a proper review. As you mentioned in your MP, the branch is just cleaning up already working JS (outside of the chromium fix - just a different class).. <noodles775> Oh, but I can hit the js review button if you need it (sorry). <jtv> I'd love that, thanks! <rinze> jtv: Yeahp! <rinze> jtv: Updating the MP now <jtv> rinze: great! Then I can still get it in before the deadline. <rinze> Also, if somebody here can review a oneliner: https://code.edge.launchpad.net/~jelmer/launchpad/fix-devel/+merge/29011 <rinze> It's the testfix for devel <noodles775> rinze: done (and jtv too) <jtv> thanks <jtv> rinze: least I can do... <jtv> oh, too late :-) <rinze> jtv: noodles beat you to it, but thanks anyway :-) * bigjools changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: - || queue: [sinzui, noodles775, bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * rinze changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: noodles775 || queue: [bigjools] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * rinze looks at bigjools branch briefly <rinze> bigjools: there's a typo in paramter in your angry comment :-) <bigjools> I was too angry to speel <rinze> in lib/lp/soyuz/scripts/tests/test_populatearchive.py nonvirtualized seems to be a boolean parameter, yet it defaults to None and checks for "is not None" <rinze> I.e. if testCopyArchiveVirtualization specified nonvirtualized=False it would still not use virtualization <bigjools> rinze: yes deliberately, but I suppose it should default to False <bigjools> I was still too angry to think :) <bigjools> both fixed <rinze> bigjools: did you push your changes? <bigjools> rinze: no, I was waiting for more comments about my angry coding :) <rinze> hehe <rinze> bigjools: Tests pass here and I don't see any other issues. <rinze> perhaps you could add an assert somewhere that self.makeCopyArchive() can also still create archives that require virtualized builds <bigjools> yeah I saw that problem as you mentioned it too <bigjools> merde <rinze> bigjools: in your latest tip, the nonvirtualized argument to makeCopyArchive defaults to None rather than False <bigjools> rinze: some changes pushed up again <bigjools> ARGH <bigjools> missed it <bigjools> ok pushed up again <rinze> bigjools: I don't see the diff for that last change yet, but I trust that's correct :-) r=me <bigjools> thanks rinze :) <bigjools> saving me from my own stupidity on that branch * rinze changed the topic of #launchpad-reviews to: On call: jelmer || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <henninge> jtv: review sent (finally) <jtv> ahhh! <jtv> henninge: hope it's an approval! <henninge> jtv: almost <jtv> grrr <henninge> jtv: I mean, there are no big issues but I'd like to hear your opinions on some of my comments. <henninge> ;-) * rinze 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 <bigjools> is anyone around for a simple but urgent review? * rockstar changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * bigjools meets rockstar here <rockstar> bigjools, I've come to the conclusion that soyuz is generally hard to understand unless you know the whole entire subsystem. <rockstar> The latter has become a new goal of mine. <bigjools> attaboy <bigjools> I still learn stuff 3.5 years after I started on it.... <rockstar> bigjools, why do you "create copy archives for rebuild testing"? That's not a PPA thing, right? <bigjools> rockstar: no, it's more like a main archive presented like a PPA. It allows us to copy source from somewhere else and rebuild it <rockstar> bigjools, presented like a PPA? What's the use case there? <bigjools> UI code re-use :) <rockstar> (Patch looks fine, just trying to squeeze a few user stories out) <bigjools> we didn't want it looking like the normal distro pages <bigjools> the main use case is to find failed builds <rockstar> Why shouldn't it look the same? <rockstar> Er, why shouldn't it look like the normal distro pages? <bigjools> they're overkill for this type of archive, mainly <rockstar> bigjools, so I guess the thing I'm not understanding is what type of archive this is. <rockstar> Is it a distro archive or a PPA or something else I don't know about. <james_w> it's a third type <rockstar> james_w, which is? <rockstar> Is it like a throwaway archive just for rebuilding copies of existing packages? <james_w> rockstar: pretty much <rockstar> james_w, ah, okay. That parses better now. <james_w> it's grown some extra features, but the primary use case is to copy sources from Ubuntu in to a new archive, and then rebuild them there <james_w> so the bug in question is where it copies a bunch of sources, and then fails while creating the build records for them <rockstar> Yeah, that makes sense. <james_w> in particular because the new archive doesn't have i386 enabled, and i386 is required to build arch "all" packages, it's the nominatedarchindep * bigjools watches james_w explain how my branch works :) <bigjools> sorry - had to dash off, child-related poo emergency <james_w> nice <bigjools> actually it wasn't :( <rockstar> Yeah, I very much understand nominatedarchindep. Remember that time that recipe builds were using the arm builder... <bigjools> unfortunately yes <james_w> damn you bigjools, I've now started typing poopy instead of poppy <james_w> rockstar: a small branch of my own if you would: https://code.launchpad.net/~james-w/launchpad/move-file-is-a-directory/+merge/29033 <rockstar> james_w, I live serve... <rockstar> Er, I live to serve <james_w> \o/ <rockstar> I apparently don't live to type correctly. <bigjools> james_w: haha <bigjools> rockstar: thanks for the review <rockstar> james_w, your branch is made of awesomeness. Thanks for doing that. <bigjools> rockstar: aw jeez you Quoted me! <rockstar> One day, we'll have all our dependencies nice and neat. <rockstar> bigjools, yeah, it had to be done. <rockstar> bigjools, also, you're like, the 6th person today to tell me about poo related mishaps that occurred today. <bigjools> one day you will have kids and will enthusiastically join in <james_w> thanks rockstar #launchpad-reviews 2010-07-02 * noodles775 changed the topic of #launchpad-reviews to: On call: rockstar || reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <adeuring> noodles775: for which branch do you need a review? <noodles775> adeuring: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-get-or-create-from-identity/+merge/28885 <noodles775> Thanks! * adeuring is looking * adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <noodles775> I asked salgado to take a brief look yesterday (as it's his area), and he's commented, but it needs a proper review. <noodles775> adeuring: also, i've got another branch that's not ready for review, but I need some help (related to private XMLRPC authentication - I'm a little unfamiliar) if you've got any input: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-priv-xmlrpc-access-getOrCreate/+merge/29059 <adeuring> noodles775: so, you want to add another celebrity? <noodles775> adeuring: that's already done (and landed by StevenK). At the moment I'm just trying to write a test that logs in as admin and passes. <noodles775> The xmlrpc call works fine (I can switch to zope.Public and the tests work), but I can't see how to assign launchpad.Admin to the xmlrpc view (or if I even should be going down that direction - I can't see any other examples in the code) <noodles775> Hrm, maybe the test should be authenticating differently, or the logged in user needs ssh keys etc. <adeuring> noodles775: as I understand it, you just need to ensure in checkAuthenitcated that the current user is your celebrity. Or am I missing something? <noodles775> adeuring: that's what I would have thought, but that breakpoint is never being called (and yet the tests return 401 Unauthorized) <adeuring> noodles775: seems that some other authentication check is kicking in then? <noodles775> Yep... I'll check if it's related to ssh keys on the test users. <adeuring> is there any auth class for the same permission and interface class? <noodles775> Nope - it's a new class in that branch. <adeuring> noodles775: after looking closer, I must ammit that I have no clue why this doesn't work... <noodles775> adeuring: ok, thanks for looking :D <noodles775> adeuring: found it... I needed to supply the credentials when creating the xmlrpc server proxy... obvious now. <adeuring> noodles775: uuhhh, yes ;) <noodles775> But I still need to find out how I can ensure other users have launchpad.Admin on the view... * noodles775 keeps looking. <adeuring> noodles775: overall, your code looks good. Just one question: SHouldn't the "with MasterDBPolicy():" be moved from processPositiveAssrtion() to getOrCreateByOpenidIdentifier()? <noodles775> adeuring: yes, you're right. <adeuring> noodles775: OK, so r=me (including aonther nitpick about a typo) <noodles775> Thanks adeuring <adeuring> On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <noodles775> adeuring: the other one is ready now if you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-priv-xmlrpc-access-getOrCreate/+merge/29059 <adeuring> noodles775: I'll look * adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <adeuring> noodles775: again, a nice branch. But I think it is a bit odd to have a class PersonSetAPIView (ie.e.,a quite generic name) where the method getorCreateByOpenIDIdentifier() unconditionally providesthe rationale "software centerpurchase" as a account creation rationale. Shouldn't we either make the class name more specific or provide the rationale as a parameter of the method? <adeuring> (or use a method name like getOrCreateForSoftwarePurchase()) <rinze> adeuring, can you review a oneliner (testfix) ? <adeuring> rinze: sure <rinze> https://code.edge.launchpad.net/~jelmer/launchpad/fix-db-devel/+merge/29070 <rinze> a warning about the completeness of the dailybuilds feature was added recently, I suspect that broke it <adeuring> rinze: well, the change looks straightforward, r=me. <rinze> adeuring: Thanks <noodles775> adeuring: yes - I thought the same when I pushed the last rev. So I'd be keen to change the application name... most of the other private xmlrpc apps have names like 'ICodehostingApplication' identifying who uses them. <bac> hello adeuring. busy review day? <adeuring> bac: quite intersting branches from noodles775 :) <noodles775> So perhaps, ISoftwareCenterAgentAPI, http://..../softwarecenteragent etc.? <adeuring> noodles775: ok, so something like s/Person/Shopper/ ? <bac> yes, i saw. looks like fun... <adeuring> ...in the app name? * bac relocates <noodles775> The app name usually matches the API, so perhaps ISoftwareCenterAgentApplication... <adeuring> noodles775: ah, right. so, that, combined with a method name like getOrCreateShopCustomer()? <noodles775> Sounds good. <adeuring> noodles775: ok, r=me <noodles775> w 5 * bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: noodles775 , -|| queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <henninge> jtv: your reply looks good but you did not push your changes. Did you rename "get_all_important_translations" ? <jtv> henninge: knew I forgot something! hang on <jtv> (Well, I didn't know you'd be this quick so technically I just hadn't gotten around to it yet) <henninge> jtv: I guess you'd like to be through with this before the game ... ;-) <henninge> jtv: also, since this branch only contains tests, it should not need a full test suite run, should it? * adeuring changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -, -|| queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <jtv> Thanks for reminding me of the game. :-) I started incredibly early today, so don't think I'll do much after. <jtv> henninge: that's the theory. Practice doesn't always follow theory. :) <henninge> jtv: one more branch after this and we can merge db-stable again? <jtv> henninge: yes, that's D.'s use-upstream-translations branch. <jtv> That will also enable us to run the "mirror" tests that I commented out. <jtv> henninge: oh, and I snuck something extra in: I defined flushing orders when disabling TMs <jtv> Push is underway. <henninge> Ah, once bitten ... <jtv> henninge: push is done, diff updated... <jtv> bac, got time for this one? https://code.launchpad.net/~jtv/launchpad/bug-181254/+merge/29056 <bac> jtv: yes, please add to topic <jtv> bac: "reviewing" or "queue"? :-) * jtv changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -,jtv || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <bac> queue, of course * bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -,foxxtrot || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <jtv> awwww <henninge> jtv: what? <henninge> oh, ok <jtv> bryceh: thanks for going the extra km in cleaning up the branch... this is to let you know that I'm approving and you should still be able to land <jtv> henninge: my branch ok now? <henninge> jtv: most likely ... ;) <jtv> the suspense is killing me! <henninge> and <henninge> the <henninge> winner <henninge> is <henninge> ... <henninge> jtv! * bac changed the topic of #launchpad-reviews to: On call: adeuring, bac || reviewing: -,jtv || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <henninge> jtv: r=me <henninge> thanks, looks very good ;-) <henninge> but now that you changed actual code, you will have to run it through ec2 :-P <bac> henninge: which jtv branch did you review? <henninge> jtv: oh, not the one he asked about today. <henninge> bac: ^ <henninge> this is three days old. <henninge> https://code.edge.launchpad.net/~jtv/launchpad/recife-limited-setcurrenttranslation/+merge/28715 <bac> oh, ok. henninge just trying not to duplicate work <jtv> henninge: thank you for your kind approval... I'm out of here! <jtv> bac: I'll keep an eye out for any questions you may have, but be aware that Holland is about to face Brazil and I may be distracted or sobbing uncontrollably. :) <bac> jtv: good luck. you'll need it. <jtv> indeed! <bac> jtv: can you keep us posted here? * bac is surprised there has not been more WC chatter <jtv> bac: ok... my first time as a commentator <bac> i watched the ger-eng game on univision. having spanish play-by-play was much less annoying <bac> gooooooooooooooooooooooooooool alemania! <henninge> YEah! <henninge> ;-) * jtv relocates to TV <henninge> jtv, danilos: Err, the potemplate api branch was never approved. <henninge> https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525371/+merge/25423 <danilos> henninge, isn't there a resubmitted MP which jtv approved? <jtv> ? <henninge> danilos: I hope there is <jtv> (wc commentary: Brazilian players looked nervous and tense... is their national anthem particularly difficult like the Argentinian one?) <henninge> adiroiban: Hi! <jtv> (the Dutch players just unfolded a large banner "say no to racism"âa superfluous statement given the diverse composition of the team) <adiroiban> henninge: hi <henninge> adiroiban: which is the MP that jtv approved for the POTemplate API? <henninge> because this one isn't: <henninge> https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525371/+merge/25423 <adiroiban> henninge: Danilo was the one doing the final review <henninge> adiroiban: hm, it was landed as [r=jtv], though. <jtv> I did approve that one, didn't I? <jtv> bac: so far the pressure went straight onto the brazilians <bac> jtv: i don't know what that means. <adiroiban> ah... yep. Danilo did approve it first... but then we decide that it requires more work and the jtv was the one to appove and land it <bac> jtv: make lint on your branch shows a lot of stuff...mostly valid i think <bac> assigned but unused variables <jtv> bac: it means the Dutch have had the upper hand so far. <bac> jtv: goals only, please. :) <jtv> bac: goals only? The violence may be more interesting... they've deliberately taken down 3 dutch players in 5 minutes and they seem to know the referee will let it go. <henninge> adiroiban: only the MP does not show that. Wonder why. But if nobody objects to you story ... ;-) <jtv> bac: those variables were in there before... they're not mine but they serve an explanatory role. <adiroiban> henninge: I guess that jtv forget to update the MP <henninge> adiroiban: yes he does forget things once in a while ... ;-) <jtv> adiroiban: I guess... I may have told you by IRC <henninge> jtv: lucky sods <jtv> henninge: they wouldn't have gotten anywhere so far without unpunished fouls... hope the game doesn't turn bilaterally ugly * henninge just looked up sods ... probably not the term I meant to choose. <jtv> bac: Brazil just scored <henninge> awwwww <henninge> jtv: come on! We have a date in the final! <jtv> I hope so... <bac> jtv: imagine a pick up match between you and stevea vs. ursula and salgado. you know how i'd bet... <jtv> bac: are you okay with those explanatory variables or do you want a cleanup round? <bac> jtv: it's up to you. i don't think they add much by way of explanation. <jtv> bac: hell, I don't want the noise... I'll take care of it. <jtv> bac: the match may not be worth following with the referee already having made up his mind <jtv> open, nonfunctional violence by the brazilians now... no yellow card <bigjools> hi bac, can you do a small branch please? <bac> bigjools: sure, next. <bigjools> oh abel is free <bigjools> I think? <bigjools> I don't mind who does it :) * bigjools files MP <bac> jtv: the comment and code don't seem to match: <bac> # Strip the last column off the result; it's just always True. <bac> 146 + return set(result) <bac> s/146 +// <jtv> bac: yikes! that comment is from an older iteration... I'll take it out <bac> thx <bac> jtv: r=me <jtv> bac: great, thanks! <bac> bigjools: r=me <bigjools> bac: easiest soyuz branch evar, thanks ) * bac changed the topic of #launchpad-reviews to: bigOn call: adeuring, bac || reviewing: -,jelmer || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <bac> bigjools: that's the truth! <jelmer_> bac_: Hi! <jelmer_> :-) <bac> jelmer_: how are we supposed to find you with a new irc nick every day? :) <jelmer_> sorry :-) <bac> jelmer_: looking at your MP, i wonder about the import for Dsc <bac> jelmer_: on our machines where this script runs do we really find Dsc in both places? <jelmer_> bac: Ah, I forgot to document that one. <jelmer_> bac: Newer versions of python-debian have renamed the upstream package from debian_bundle to debian <jelmer_> it still provides debian_bundle but will raise a DeprecationWarning <jelmer_> that warning causes various tests to break <bac> jelmer_: and our machines are in flux between the two? <bac> or is it a dev vs production issue? <jelmer_> bac: Maverick has the newer version of python-debian, and we'll hopefully include a more up to date version in the PPA as well soon. <bac> jelmer_: ok, but eventually we'll all find it in one place. how about an XXX comment for cleaning up the import when things stabilize? <jelmer_> bac: That's a good idea. <bac> thanks jelmer_ . r=bac <jelmer_> bac: Thanks! <bac> hey bigjools it'd be good to tell your reviewer if you're looking for an RC. it really shouldn't matter but i would look at things much harder if i knew it was an rc candidate. <bac> otoh, i should've asked... <bigjools> well, I don't understand how that affects the review <bac> bigjools: ideally it shouldn't. <bac> they should all be super stringent <bigjools> but I should have said - in fact I normally do <bigjools> so sorry 'bout dat <bac> but in reality i know i'd pay a lot more attention <henninge> bac: goooooooooooal! * bac changed the topic of #launchpad-reviews to: bigOn call: adeuring, bac || reviewing: -,bjornt || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <henninge> bac: gooooooooooooooal! <henninge> NED - BRA 2:1 <henninge> jtv is probably dancing Samba ... ;-) <jtv> henninge: good thing I'm not on Mumble <henninge> yeah, it'd probably burst my earphones <jtv> My God, that was a deserved red <henninge> I say <jtv> Stabbing your cleats right into the back of a prone man's leg... <jtv> Damn, that red card should've gone to Caca <henninge> jtv: that was yellow <jtv> yes, but what Caca did deserved red. <henninge> jtv: did you realize that we will be watching the final together in Prague? <jtv> henninge: I'm being very careful in my use of my country's very rich vocabulary for your countrymen ;-) <jtv> Would be fun to watch de-nl together! <henninge> What I expect! ;) <henninge> They need higher goals for those kind of shots ... <jtv> We got lucky there... <jtv> ...and not there <henninge> jtv: Congratulations! <jtv> henninge: thanks! Now you do the same and we'll meet in Prague! <henninge> jtv: We'll meet anyway ... :-P <jtv> :D <jtv> henninge: Sneijder just explained why he kept shooting into the goalie's arms: he knew the guy isn't so good at catching close to his body. <jtv> The surprising thing is I don't hear any vuvuzelas outside... Excellent noise isolation my dad has. <jtv> bac: Holland passes 2â1, deservedly. Loyalty aside I was sort of rooting for Brazil until the first minutes when the fouls started. <bac> jtv: nice <henninge> jtv: enjoy your weekend! I'll be going there now, too. ;-) <jtv> henninge: thanks, you too... have a lemonade on us! <henninge> I will ! ;-) <bryceh> jtv, excellent, thank you! <jtv> bryceh: whoa, took me a while to figure out what that was about. :) * adeuring changed the topic of #launchpad-reviews to: On call: bac || reviewing: bjornt || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <jtv> bac: apparently the Brazilian fans were good sports and many said the Dutch team was simply better in the second half. I think the referee let a few Dutch fouls slip later on to compensate for some really bad ones he let go from the other side earlier. <jtv> I wish we'd get instant replay soon... bad calls breed lots of resentment on both sides that they take out on each other. * sinzui changed the topic of #launchpad-reviews to: On call: bac || reviewing: bjornt || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <sinzui> Hi bac. I have a branch for review. My apologies for the diff which I think is hard to read because of the many small changes needed to expose summary and copyright for packages. <bac> sinzui: ok * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: sinzui || queue: [bjornt] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <bac> sinzui: i get errors in test_highlighted_copyright_is_None <sinzui> :( <sinzui> I get them too <sinzui> one moment <sinzui> ah <sinzui> bac: The setup is wrong. The copyright must be set to None. Our factory is better than our data, or some of our uploaders <sinzui> bac: this is the fix for when there is no copyright file: http://pastebin.ubuntu.com/458417/ <bac> sinzui: if you go to https://launchpad.dev/ubuntu/warty/+source/a52dec you'll see a warning that there is no current release <sinzui> yes, that is true <bac> clicking on the +changelog gives a nice but empty page <bac> +copyright oopses <sinzui> bac: sample data is very very bad <sinzui> remember that we have to use the factory to make SPPH for everything, that page is ehy <bac> so if it were fully fleshed out the oops would not happen <sinzui> pmount and mozilla-firefox have some data <bac> sinzui: ok. i just want assurance that we're being suffiently defensive <sinzui> The page told the truth about no release, and we do not make links to impossible packages <sinzui> however, I will make copyright not oops when someone is being bad <sinzui> bac: this is the fix to see info that there is no copyright: http://pastebin.ubuntu.com/458422/ <bac> great <mars> sinzui, ping, when you have a moment, could you please review https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116 ? <sinzui> mars: I do not think that will work <mars> neither do I <mars> '|' is not OR, is it <sinzui> mars tales is testing None or something <sinzui> False is something <mars> eewww, even worse * sinzui thinks if there is a tales formatter for false>None <mars> surely there is some way to just do a binary expression in TALES? <sinzui> no <sinzui> but you can do: <sinzui> python: is_edge or is_lpnet <mars> do the variables come through? <mars> from TALES back to Python ? <sinzui> They do in this case I believe. We scoped them to the entire template <mars> simple, testing now <sinzui> mars, if not you need to do python: config.launchpad.is_edge ?? <mars> sinzui, python: is_edge or is_lpnet works <sinzui> rock <mars> branch scanner is slow today <mars> pushed, just waiting for the update <mars> updated <mars> sinzui, r=you? <sinzui> mars: yes r=me <mars> I wonder if 'ec2 land' understands the release-critical tag <mars> thanks sinzui <mwhudson> mars: it adds [release-critical=] if there's a review with that tag iirc * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [bjornt] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <bac> mwhudson: that is correct <mars> cool <mars> sinzui, could you please hit 'Approve' on the merge proposal? https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116 * bac 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 <mars> bac, care to give a second a review of https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116 ? <bac> sure <bac> mars i'm confused <bac> you reviewed your own branch? <mars> bac, I don't know how to formally say 'Curtis reviewed it' - there is no Proxy Review button <bac> mars: why didn't he mark the MP? <mars> I can vote by proxy - why not review by proxy as well? <bac> the commit message is going to be a hoot <mars> :D <bac> [r=mars][rc=mars][bug=mars] <mars> bac, I don't know, he isn't answering <bac> i would actually suggest you get an rc from curtis since he was the last RM. i'd be very uncomfortable giving myself an rc <mars> bac, I can do the r-c myself <bac> you certainly can. i think it is a bad idea. that's just me. <mars> you know, by being Release Manager and all <mars> true, but that is why I asked for your second code review <mars> from a RM perspective, this has to land to fix the lp_lucid builder <mars> so there is no question around the RC <mars> bac, ok, Curtis reviewed it - could you still take a look at the code, just to make sure it's sane? <bac> mars: doing so now <mars> thanks <bac> mars, what about staging and dogfood? <mars> bac, no, we do not want to track the usage of those servers. They will just pollute the data. <mars> Since they are test environments <bac> mars: fair enough. r=me <mars> thanks bac #launchpad-reviews 2010-07-05 * henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <noodles775> Hi henninge, would you be able to review an MP of Jelmer's for us pls? https://code.edge.launchpad.net/~jelmer/launchpad/archive-arch-fix/+merge/29160 * henninge looks and considers ... <henninge> :-P <noodles775> :) <henninge> noodles775: yeah, ok ;) <noodles775> Great, thanks! * henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: jelmer || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <henninge> noodles775: so, do I talk to you about the mp or shall I just reply by mail to jelmer so that he can process it? <noodles775> henninge: Best to do it on the MP if that's ok. <henninge> yes, find <henninge> e <henninge> dine <henninge> fine <henninge> ;) <noodles775> lol * henninge_ changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <henninge> noodles775: reviewed but not yet approved. <noodles775> henninge: thanks for that. <bigjools> henninge: I'll take over <bigjools> wasup? <henninge> bigjools: I replied in the MP <bigjools> henninge: what's the URL for that please? <henninge> https://code.edge.launchpad.net/~jelmer/launchpad/archive-arch-fix/+merge/29160 <bigjools> henninge: what are you referring to with "I see more instances of that" ? <bigjools> oh the dodgy docstrings? <henninge> right <bigjools> henninge: replied <henninge> cheers * henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: lunch || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * StevenK changed the topic of #launchpad-reviews to: On call: henninge || reviewing: lunch || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <bigjools> henninge: you really want another MP? <henninge> bigjools: no, I just thought you'll need it to request the r-c, don't you? <henninge> cp, not r-c <bigjools> henninge: I can do that on the existing one <bigjools> it's for r-c <bigjools> so it goes in this release <henninge> oh, right, we release <henninge> ;) <bigjools> :) <henninge> bigjools: as you like it ;) <bigjools> that restricted architecture stuff that was done as a while ago was a total shambles <bigjools> this is not the first fix to what you'd think would be an obvious bug <henninge> uh-oh <henninge> I am still -1 on the (actual, expected) thing, btw. <bigjools> it reads so much better to me that way :/ <bigjools> then it's more like how we write code with a '==' operator <mwhudson> you could always use assertThat(value, Equals(expected)) :-) * henninge changed the topic of #launchpad-reviews to: On call: henninge || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <adiroiban> jtv, henninge hi, I have updated the branch for POFile statistics, to export them as a dedicated object https://code.edge.launchpad.net/~adiroiban/launchpad/bug-583934/+merge/26965 <jtv> adiroiban: thanks, I'll take a look right now <adiroiban> i had to create a canonical_url for the new object <adiroiban> not sure if we want that <adiroiban> or we want to create StatisticsObject for each statistics type <adiroiban> or a single StatisticsObject that will handle statistics for POFile, DistroSeriesLangauges, SourcePackages * StevenK pouts at adiroiban jumping the queue :-P <adiroiban> StevenK: sorry for that. This is a pre-imp chat , not a real MP <StevenK> adiroiban: It's all good :-) * abentley changed the topic of #launchpad-reviews to: On call: henninge, abentley || reviewing: -,- || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <jtv> adiroiban: what you did looks right to me (though I'm not a zope expert) though I wouldn't special-case the "statistics" string in the navigation class. Wouldn't +statistics (or +stats) be a cleaner way to solve this? <adiroiban> jtv: afaik, +somethins is for describing a view <adiroiban> not an object <jtv> adiroiban: frankly I've always wondered about that. :) <adiroiban> also, I don't know how to avoid the special casing on the URL traversal <jtv> True, you'll definitely have to have _some_ special-casing if we don't want to rearrange our URLs <adiroiban> jtv: what I am not sure that I did right, is the exposure of TranslationStatistics objects only in the API and not in the browser view <adiroiban> I am not sure if this is ok <adiroiban> or the rule is to have a direct match between API and the browser views <jtv> adiroiban: I don't know of any such rule... maybe someday we'll find it useful to attach a small view to this but that's for later I think. <adiroiban> ok <adiroiban> then I will go on and polish the code <StevenK> henninge, abentley: Which one of you wants to look at my MP? :-) <henninge> StevenK: already lookin <henninge> g * henninge changed the topic of #launchpad-reviews to: On call: henninge, abentley || reviewing: StevenK,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <adiroiban> jtv: there is one more think I would like to ask. in model.POFile.translation_statistics , it creates a new statistics object on each call. Would it not be better to create a single statistics object in POFile.__init__, and retrieve it as a singleton? <jtv> adiroiban: I don't think it's worth it. <jtv> We try not to cache things in ORM-backed classes. <adiroiban> jtv: for the future, I was thinking that we can move all translation statistics methods into this new object <lifeless> think + is used for many things <lifeless> +text, for instance <jtv> adiroiban: that still won't make it very expensive to create one of these objects, and bear in mind the object only lives for the duration of the request so it's not taking up space across requests either. I just don't think it's a problem. <adiroiban> jtv: ok :) <jtv> We have some other classes like this, like SourcePackage. <adiroiban> jtv: thanks. than I'm good. I will come back with a real MP :) <jtv> adiroiban: great, thanks! <adiroiban> lifeless: are you saying that I can use +statistics to retrieve an object and then have something like IPOFile/+statistics/+statisticsview ? <lifeless> maybe <lifeless> I'm going to try sleeping again :P gnight <adiroiban> :) <adiroiban> good nigh <StevenK> henninge: No news is good news? <henninge> StevenK: sorry but most likely yes ;) <StevenK> henninge: Assuming silence to mean 'r=<nick>' is fraught with peril. And most likely, a lynching. <henninge> StevenK: yes, I got distracted but I am preparing my reply now. Hang in there. <henninge> StevenK: The test uses a celebrity for software-center-agent but the test uses this: <henninge> > + >>> celebrity = factory.makePerson(name='software-center-agent') <henninge> Or am I missing a hidden functionality of makePerson that will return an existing person if the name matches? <StevenK> henninge: software-center-agent doesn't exist in the sampledata <henninge> But how can it be a celebrity, then? <StevenK> The celebrity look up is done via name <StevenK> So Julian and I thought it easier if the tests that needed it (of which there are about 4) just created it <bigjools> celebs don't need to be in the sampledata <henninge> I thought they do ... <bigjools> but StevenK does need to write a database patch to add it to production ;) <StevenK> I do? It already exists in production <henninge> bigjools, StevenK: But is it not bad practice to do something like getUtility(ILaunchpadCelebrities).software_center_agent when that is not defined in the interrface? <bigjools> henninge: what interface? <StevenK> The interface code calls that <StevenK> henninge: It only does so for commercial P3As <jtv> abentley, care to review this tiny branch? https://code.launchpad.net/~jtv/launchpad/recife-test-fixup/+merge/29216 <abentley> jtv, sure. <abentley> jtv, what does "recife" mean? <jtv> abentley: thanks. Recife is a feature branch we're working on (after the location of the corresponding sprint). <abentley> What does it mean? <jtv> It is the location of the sprint corresponding to the feature we're working on. <abentley> jtv, I'm sure it won't be confused with "recipe" at all :-P <abentley> jtv, Why does being a feature branch mean that EC2 is no help? <henninge> StevenK: review sent, r=me but with a little uneasiness. <jtv> abentley: well, "ec2 land" is no help. <jtv> abentley: Thinking I could just grab Henning, I failed to explain much about the change. A template can be attached to either a ProductSeries or a DistroSeries + SourcePackageName. In a review change to a test I wanted to make a template as a "cousin" to an existing template. But instead of creating it for a distroseries + sourcepackagename like the existing one, I created it for a productseries. <abentley> jtv, I see. <abentley> jtv, r=me. <jtv> abentley: thanks * henninge changed the topic of #launchpad-reviews to: On call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <bigjools> jtv: so you're using code names named after the sprint location too huh? <bigjools> this is a good trend. I want a Maldives project. <jtv> bigjools: not as such, but coming up with a terse description of the feature somehow never became much of a priority with such a catchy name. <bigjools> jtv: we started tagging bugs "wellington" remember :) <mwhudson> tahiti would be ok, too <bigjools> as would tortola <bigjools> which is a really cool name <jtv> Read the draft treaty the global-warmists were working out in Kopenhagen... Bali meeting, Bangkok meeting, Hawaii meeting... really handy. <bigjools> there's certainly some increased air temperature at those meetings * abentley 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 #launchpad-reviews 2010-07-06 <jtv> henninge: do you want this one? https://code.launchpad.net/~jtv/launchpad/recife-another-testfix/+merge/29262 <henninge> jtv: anything to get this merge done ... ;) <jtv> henninge: the fix should work equally with or without the merge, so should be easy. <jtv> hi danilos <danilos> jtv, hi <henninge> danilos: hi ;) <henninge> jtv: you are talking "product series" but the change is about "distro series". Why is that? <jtv> henninge: probably a simple slipup... where exactly? <henninge> jtv: in the mp description <jtv> henninge: oh, then it's definitely a typo. Finger memory. <henninge> ok <henninge> let me just verify that htis helps ... ;) <jtv> As you would have guessed from factory.makeProductSeries(distribution=ubuntu) :-) <henninge> jtv: great. it works! <henninge> jtv: r=me <jtv> henninge: dankeschÃ¶n! <henninge> jtv: will you merge it into recife right away? <jtv> henninge: it's currently pushing <henninge> great! :-D <jtv> It Is Pushedâ¢ <henninge> I just remembered that there were also windmill tests failing. <jtv> henninge: got a log? <henninge> jtv: yes, it's in the same <henninge> http://people.canonical.com/~henninge/merged-lp.translations.tests.log * gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: lunch || queue: [] || RC branches jump the queue || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: - || queue: [] || RC branches jump the queue || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * bigjools changed the topic of #launchpad-reviews to: On call: gmb || reviewing: - || queue: [bigjools] || RC branches jump the queue || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * bigjools waves at gmb <gmb> Hullo <gmb> bigjools, Diff me. <bigjools> I just 'bzr send'ed the MP, it should appear shortly <gmb> RIghto <bigjools> https://code.edge.launchpad.net/~julian-edwards/launchpad/oops-dspr-page-bug-592417/+merge/29273 <gmb> bigjools, r=me * gmb changed the topic of #launchpad-reviews to: On call: gmb || reviewing: - || queue: [] || RC branches jump the queue || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <bigjools> thanks gmb. And see yer next week. <gmb> Would've been quicker but I had a tea-based emergency <gmb> bigjools, Welcome. And likewise, unless I go blind in the interim. <bigjools> as in you needed more <bigjools> or as in you spilt it <gmb> bigjools, It had gone cold. <bigjools> oh disaster <gmb> Indeed. Calamity, oh woe is me, etc. <gmb> Thankfully, it has now been refreshed. Hurrah, etc. Let joy be unconfined. <bigjools> and on that note I need coffee. Some Finca el Fany awaits. <noodles775> bigjools: I've marked the mp as ui=me, but with a small suggestion - see what you think. <bigjools> noodles775: ok cheers <bigjools> noodles775: see https://dogfood.launchpad.net/ubuntu/+source/uex/156.14.161.142-1/+index <bigjools> with that listing change <bigjools> I agree that the old way looks a bit odd, but this way looks more odderer :) <bigjools> noodles775: a third option to add to your suggestions would be to centre the data across the three <td>s <noodles775> bigjools: there are other class (and class combinations) for tables in the stylesheet(s)... I'd recommend going with one of those (as they're what have been developed for consistency). <noodles775> I think centering the data inside the td's would be odderer still ;) <bigjools> true <bigjools> :) <bigjools> noodles775: remind me where the css files live .... <bigjools> ah found 'em <noodles775> Great. <bigjools> noodles775: unfortunately nothing looks that good * noodles775 has a look at product and other pages. <noodles775> bigjools: every table I can find is using the listing class? <noodles775> That doesn't mean it looks good ;), but does mean that it would be best to use it and if we later improve the look of table.listing, they'll all be improved together... or is it something particular about this instance that you didn't like? <bigjools> noodles775: ok what about now? <noodles775> What don't you like about it? (and definitely not if you're going to land this RC ;) 0. <bigjools> interestingly FF doesn't render the end of the "Size" heading cell marker <bigjools> it's too wide <bigjools> it only needs to fill the space it needs <bigjools> IMO <noodles775> That's what the 'narrow' class is for in the stylesheet... not sure why it's not doing anything though. <bigjools> there's a "compressed" as well but it doesn't do anything either <bigjools> noodles775: aha, check it now <bigjools> not sure about the "-" characters <bigjools> maybe just leave it blank <noodles775> hmm... it's still wide for me (I've shift-refreshed). What classes did you add? <bigjools> noodles775: it renders differently on Chrome and FF :/ <noodles775> According to the chromium inspector, the 'narrow' isn't doing anything... <bigjools> "narrow listing" <noodles775> ah. * noodles775 looks in FF * bigjools shakes fist at browsers <noodles775> bigjools: so right, I think you've found the right classes, and up to you whether you leave the '-' in or not. <noodles775> Might be worth checking with sinzui or creating a bug about the 'narrow' class seemingly being ignored on chromium. <noodles775> (just in case he already is aware of it). <bigjools> yeah, I'll stick with "narrow" then and file the bug <bigjools> cheers * mars changed the topic of #launchpad-reviews to: On call: gmb, mars || reviewing: - || queue: [] || RC branches jump the queue || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews * sinzui changed the topic of #launchpad-reviews to: On call: gmb, mars || reviewing: - || queue: [sinzui] || RC branches jump the queue || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <sinzui> gmb, mars: I have a branch that removes lots of crufty lint checking <mars> sinzui, I'll take it * mars changed the topic of #launchpad-reviews to: On call: gmb, mars || reviewing: -,sinzui || queue: [sinzui] || RC branches jump the queue || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <mars> sinzui, out of curiosity, does pylint have a test suite? <mars> specifically, a test suite with sample data <sinzui> I do not think it does <sinzui> It does not have a public bug tracker <mars> sinzui, does your linter choke on conflict markers? '<<<' and '>>>' <sinzui> no <mars> does it flag them? :) <sinzui> It looks for 7 start or close, which is better than what we had <mars> 7 start or close? <sinzui> mars: our linter reports the start of a conflict marker. pocketlint report the start and the end <mars> ah, ok <mars> sinzui, why was jslint was removed from the list of checks? Do we have a backup? <sinzui> pocket-lint uses spider-monkey too <sinzui> it however reports problems consistently with all the other linters <mars> sinzui, a simple test should verify that - remove a semicolon from a JS file, see if it catches it <sinzui> yep <mars> I did not even know we had this lp-verbose.py stuff <sinzui> I do <mars> it's like rummaging through an attic <sinzui> You could run lint.sh manually to do a serious analysis of modules to locate duplicate code <mars> sinzui, reviewed * mars changed the topic of #launchpad-reviews to: On call: gmb, mars || reviewing: -,- || queue: [sinzui] || RC branches jump the 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: [] || RC branches jump the queue || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews <sinzui> thanks <sinzui> mars, python-pocket-lint is a launchpad-developer-dependency. Did you update today and do you see that we guarantee the file? <mars> update-manager died with a system crash, so no, I missed that :p <mars> How ironic is it that change that come from our PPA are not "a source that supports changelogs" <mars> sinzui, ok, got it, thanks. You may want to tell the lp-dev list after this lands. <bigjools> mars: hysterical raisins <sinzui> mars, I certainly will. I get to end two action items from the reviewers and team lead meetings
{'channel': 'launchpad-reviews', 'month': 7}