#launchpad-reviews 2010-04-01 <jtv> wgrant: how does the self.home[1:] work? Just strip off the leading slash to get a relative path? <wgrant> jtv: Right. There's probably a better way to do that, though. <jtv> wgrant: my first choice would probably have been re.sub, but I guess a key point is that there's nothing sensible to do in cases where it's a relative path. So I'm thinking an assertion may be enough to turn weird "known path with missing letter" errors into clearer ones. <jtv> o-kayyyy... feature request for lucid: plug-n-play USB mouse support. <wgrant> jtv: It works. Restart X -- you probably had an incompatible evdev driver update. <wgrant> So it will have tried to dynamically load the driver, and failed. <jtv> I'll do without for now. The macbook pro has a wonderful huge, buttonless touchpad. Now if only the "disable touchpad while typing" worked a little more reliably. :) <jtv> wgrant: there is a test for updateBuild_WAITING... I'd guess that all it needs is a tarball to upload and you'll have an integration test that exercises the _uploadTarball fix. <wgrant> jtv: Oh, I forgot you actually had useful tests. <jtv> \o/ <wgrant> Soyuz is rather deprived. <jtv> Rather. :) <jtv> It's funny how that seems to be infectious. Sometimes in Wellington I heard things like "we'll know when it breaks." But me, I often find myself stumped by _how_ it breaks when it does. <jtv> Have a look at lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py, test_updateBuild_WAITING <wgrant> jtv: That explicitly clobbers _uploadTarball. <wgrant> It sounds like I'm going to need to set up a real branch with translation-capable projects attached. <jtv> wgrant: hmm... yes, that sounds very true. <jtv> I don't know of any good way to inject "pretend this is the project we're looking for" into the utility. * wgrant doesn't actually know how Rosetta works. <wgrant> It's the one part of LP that I remained completely ignorant of until Saturday. <jtv> wgrant: you could consider cross-breeding this test with an existing one for _uploadTarball. <jtv> wgrant: then welcome! <wgrant> jtv: Is there an existing one for _uploadTarball? <jtv> Has to be... <jtv> test_uploadTarball may be our baby; same file. <wgrant> Oh. That's conveniently short. <jtv> Most of the meat is in the setup. :-) <wgrant> yeah. <wgrant> Any hints? <jtv> You need all of the setup. <jtv> Whereas the updateBuild_WAITING test does not rely on any implicit johncrosby@example.net. <wgrant> Although it does need makeBehavior. <jtv> So I'd create the integration test by copying test_updateBuild_WAITING right behind the _uploadTarball test. <wgrant> Ah, but that's in the mixin. <wgrant> Didn't notice that. <wgrant> That's easy, then. <jtv> yay for mixins :) <jtv> It's already in the ancestry of the test class we're looking to extend, I believe. <wgrant> Exactly. <jtv> So you can copy test_updateBuild_WAITING in there, remove all the assertions, and tack on the final paragraph from test_uploadTarball. <wgrant> Yep. <noodles775> stub: Do you know if there is any performance benefit to using CREATE TABLE AS SELECT... to create and populate a table from *lots* of existing data, as opposed to the normal method of create table, then a migration function to iterate and insert lots of data? <wgrant> Pfft, it's only 1.6 million rows. <noodles775> Just interested :) <stub> inserting rows one at a time is slower. Inserting them in bulk is faster. Building the indexes after populating the table with data is a win. <noodles775> Thanks. <stub> CREATE TABLE AS SELECT is the same as CREATE TABLE; INSERT INTO [...] <noodles775> stub: except that the indexes are added afterwards right? (and the INSERT INTO is * 1.6 million for the data migration). <noodles775> stub: here's a very early WIP that I was toying with: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-build-generalisation-db-changes/+merge/22527 <stub> Indexes are created when you create them. You might create them as part of the CREATE TABLE statement, or you might create them after. <noodles775> Right. <noodles775> stub: OK, so there's no difference between the above create table as select... and the equiv. create table, followed by a function that does 1.6 million inserts into those tables, as long as the indexes are created afterwards in both cases? <noodles775> s/tables/table <stub> Yup. But 1.6 million doesn't take long anyway unless the rows are particularly wide. <noodles775> Yep. Thanks for the info! <noodles775> jtv: I know it should be the other way around, but do you mind r or rs'ing a trivial testfix for the db-devel failure: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-test-fix-timebomb/+merge/22595 <jtv> noodles775: hadn't noticed the failure, but hang on, I'm looking <noodles775> Thanks. It's what's holding up devel being re-synced after the rollout. <jtv> noodles775: when you say it should be the other way around, do you mean I/we broke it? <noodles775> jtv: no, I should be reviewing right now :/ <noodles775> But there's just been a whole bunch of un-expected things, so I haven't started yet. <jtv> noodles775: well you couldn't have reviewed your own, and I don't recommend sitting around waiting for someone else to fix it either. :) <jtv> same here <jtv> noodles775: one question: doesn't this risk undermining the test? A ... could be hiding an entire line of output. <noodles775> Yes, if an extra line suddenly appeared in the output, it would be swallowed. <noodles775> Any other easy fixes? <jtv> Either leaving the date out of the output altogether, I guess, or replacing it with a yes/no <noodles775> OK, doing so now. <jtv> thanks for bearing with me. <jtv> wgrant: any luck with that test? <wgrant> jtv: Sorry, got distracted. <wgrant> Yes, it works. <jtv> cool! * jtv always feels a warm glow whenever integration-testing of the build farm in the test suite is extended <jtv> wgrant: not showing up in the diff yet though <wgrant> jtv: No, just pushed now. <wgrant> I wrote it, fired off the tests, and forgot. <jtv> wgrant: you're just not pushy enough :P <wgrant> Heh. <jtv> (I assume there's an implicit "committed" in that story) <wgrant> Yes. <jtv> Just being pedantic, never mind me anyway <wgrant> jtv: Can you think of a way to test the slave change? <wgrant> I'm not sure of what tests you have around there. <jtv> wgrant: maybe you could do a test where you fake addWaitingFile, and see that it gets a sensible path? <jtv> Feel free to extend FakeMethod to record its last args and kwargs. <wgrant> jtv: I was hoping there was existing infrastructure around there. * jtv searches <jtv> Not really infrastructure, but if you extend FakeMethod in this way, then lib/canonical/buildd/tests/test_translationtemplatesbuildmanager.py already fakes gatherResults for testingâshould provide a usable base. <noodles775> jtv: I'm push the changes, but the MP hasn't updated yet. <jtv> noodles775: thanks <noodles775> jtv: http://pastebin.ubuntu.com/407482/ <noodles775> That's the incremental. <jtv> noodles775: approved. <noodles775> jtv: Thanks. * noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * noodles775 changed the topic of #launchpad-reviews to: on call: noodles775 || reviewing: wgrant || queue: [jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * ctrlsoft changed the topic of #launchpad-reviews to: on call: noodles775,jelmer || reviewing: wgrant, jtv || queue: || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * noodles775 changed the topic of #launchpad-reviews to: on call: noodles775,jelmer || reviewing: mvo, jtv || queue: || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * wgrant changed the topic of #launchpad-reviews to: on call: noodles775,jelmer || reviewing: mvo, jtv || queue: [wgrant,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <mrevell> Hello. I have a fairly simple text change branch, if anyone would like to review it! https://code.edge.launchpad.net/~matthew.revell/launchpad/maintenance-notice <noodles775> Sure! Just add it to the queue pls. <noodles775> wgrant: would you have time to go over a review I'm doing and adding any comments (I'll be finished in a minute, but I'd like a second opinion from someone more familiar with germination etc.) <noodles775> s/adding/add <noodles775> And it doesn't need to be today. <wgrant> noodles775: Sure. mvo's? <noodles775> wgrant: thanks, yep. I'll be finished soon and will add you as a reviewer. * ctrlsoft changed the topic of #launchpad-reviews to: on call: noodles775,jelmer || reviewing: mvo, wgrant || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <ctrlsoft> hmm, this is my third week of reviews and so far I've only done soyuz reviews for some reason :-) <noodles775> ctrlsoft: yeah, and active reviews is all soyuz stuff too! <ctrlsoft> wgrant: do you need me to land move-lots-of-buildd-master-to-queuebuilder ? <noodles775> wgrant: ok, I've posted my comments and requested a review from you. <wgrant> ctrlsoft: When PQM opens, that would be lovely. <wgrant> For now, maybe just mark it Approved so it drops off +activereviews. <wgrant> Thanks. <wgrant> Ah, you already have. * noodles775 changed the topic of #launchpad-reviews to: on call: noodles775,jelmer || reviewing: mrevell, wgrant || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * noodles775 changed the topic of #launchpad-reviews to: on call: noodles775,jelmer || reviewing: lunch, wgrant || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <wgrant> noodles775: I don't like the way it uses python-apt when it could just use internal LP APIs, but I guess that's reasonable given that it's not really part of LP. <wgrant> Otherwise it's fine. <cjwatson> noodles775: I'd like to work on fixing bug 294846, since it's annoying me. Do you have a moment for a pre-impl chat? <mup> Bug #294846: Setting to Won't Fix is ACLed but unsetting it isn't <ubuntu-qa> <Launchpad Bugs:Triaged> <https://launchpad.net/bugs/294846> <wgrant> Is it actually desirable? It's unprecedented. <cjwatson> How so? We have existing ACLs <wgrant> Nothing that prevents users from rectifying triage mistakes and reopening bugs. <wgrant> (there's of course the obvious problem that that sometimes isn't wanted, but...) <cjwatson> The current pattern is: developer says they don't intend to fix a bug, and rather than trying to persuade them that they're wrong in comments, the user just slams it back to New <cjwatson> repeat until very bored <wgrant> Well, there's discussion about introducing a new status for that, which looks less obviously closed. <wgrant> So users will hopefully not try to reopen it. <cjwatson> That seems like a silly workaround to me <wgrant> Unless Mark starts Won't Fixing my bugs without comment again :P <cjwatson> users will sooner or later figure out that it means closed, and then we're back where we started <wgrant> Hmm. <cjwatson> reopening bugs rather than engaging in discussion with the developer is a community anti-pattern <wgrant> Indeed. <cjwatson> the Debian BTS even says in the mail you get when a bug is closed that if you think it's wrong you should discuss it with the developer, who'll reopen if needed <cjwatson> that said I don't think people should be prevented from reopening Fix Released - Won't Fix is different because it's generally the result of a genuine difference of opinion, in which case simply reopening it is unlikely to be useful <cjwatson> still, if it's controversial, then I suppose a pre-impl chat is premature <wgrant> https://dev.launchpad.net/LEP/CloseBugsLeaveDiscussionsOpen <cjwatson> aha <cjwatson> bizarre summary though. Discussion remains open on Won't Fix bugs too. <cjwatson> I'll just link to that from the bug, then <wgrant> Yes, I thought that a little odd too. <cjwatson> distinction between socially open and technically open, I suppose <cjwatson> While I'm here, is it reasonable/acceptable/common to submit Launchpad patches having run *only* EC2 tests, not local tests? I find that I don't have the resources at the moment to keep enough local infrastructure for running Launchpad tests, since I do it only occasionally <wgrant> Not common, but if it passes EC2 then it's fine... but that sounds like an inefficient way of running tests unless your change is tiny and you're really sure. <cjwatson> mm, it tends to take me at least an hour to clear enough space for Launchpad tests, update all the dependencies and such, and then I'm less efficient at whatever else I'm meant to be doing <cjwatson> if I were doing it all the time then that would be different <wgrant> Yeah. <noodles775> cjwatson: is it worth updating that LEP or chatting with jml regarding the bug? (at least so the LEP indicates why the decision has been made to allow unsetting from Wont Fix or similar)? * noodles775 can't see a comment related to it there. <jml> we hadn't even thought of ACLs for the status change * jml makes a note <jml> wgrant, cjwatson, I've just commented on the bug. I think the ACL on wontfix is separate from the OPINION status discussion. <deryck> interesting discussion. * deryck johncrosby@example.net.... <jml> deryck, oh yeah :) I was surprised to find a discussion like this on #launchpad-reviews <deryck> yeah, I don't watch this channel unless pinged ;) <jml> but I'm firmly stuck in my "fewer channels" heresy <jml> deryck, the keyword highlight on "bug" not working out for you so well? :P <deryck> So I fall in the camp of bdmurray and cjwatson on this. The ACL is pointless when unset. I didn't realize we had this bug even. Sorry. <deryck> jml, I droped that after 4 hours ;) <jml> it's been pleasing to me watching my "package branch" keyword watch become increasingly useless as people use them more. <deryck> heh <deryck> jml, I take it you favor the ACL on unsetting WONTFIX as well? <jml> deryck, yep. <cjwatson> I was working on http://paste.ubuntu.com/407546/, but then it occurred to me that I probably ought to talk about it with somebody, and I hadn't got close to testing it yet <jml> deryck, wgrant has a good point that it makes it gardening mistakes harder <cjwatson> just means you have to find somebody minimally competent to fix it for you ... <jml> deryck, I can't think of how to resolve that tension though. <jml> cjwatson, for Ubuntu, sure. <cjwatson> mm, true <cjwatson> (I wouldn't have brought it up in this channel if I'd realised it was controversial, sorry) <cjwatson> do many other LP pillars have Ubuntu's problem of simply being unable to keep up with bug mail? <deryck> jml, yes, I definitely see wgrant's point. but it's a "mistake" of semantics, though. Do bug supervisors regularly accidentally set something to WONTFIX? <deryck> or is it a matter of "no, you're wrong, this should be fixed." :-) <jml> cjwatson, too much in LP is controversial. some strange alchemy makes them less controversial once they are done though. <deryck> cjwatson, I don't think anyone has the level of problem Ubuntu has. Both others certainly complain about the mail. <jml> deryck, you mean "LP cannot be expected to tell the difference between an accidental status change and a genuine difference of opinion"? <deryck> jml, exactly. <jml> deryck, I'd agree. How do users discover who has bug-triaging powers for a given pillar? <cjwatson> if accidental setting to WONTFIX is a concern, then leave it out of the AJAX thing? <cjwatson> (not that that wouldn't be confusing in other ways ... but most of the accidental changes seem to be AJAX mis-clicking) <jml> cjwatson, tbh, I don't think it's a major concern. <deryck> cjwatson, I'm also suggesting that I don't think there is much if any accidental setting of WONTFIX. bug supervisors aren't generally careless. <deryck> jml, ^^ <jml> agreed. <jml> ok, I'll drop it. <deryck> I think if a user disagrees and we have this ACL preventing unsetting, then a user should add a comment: "hey, you dofus this should be fixed." :-) <deryck> hyperbole, but you get the idea. <cjwatson> deryck: while I'm generally in your camp, there's quite a lot of bug triager carelessness in Ubuntu, unfortunately <cjwatson> (but this is a problem we should fix!) <deryck> cjwatson, ah, true. You have a large group of people doing this. <cjwatson> and a social misdesign in which triage is marketed as something that's an easy way to start helping out Ubuntu <deryck> agreed! <cjwatson> but I could rant about this for hours <deryck> every project does that. and it's sooooo bad. <deryck> I wonder, too, if it's not self correcting, i.e. once people realize they can't easily unset it if bug traigers become more careful. * noodles775 changed the topic of #launchpad-reviews to: on call: jelmer || reviewing: wgrant || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jml> hmm <jml> that's given me a completely irrelevant idea <deryck> cjwatson, so I would welcome the branch here for this. I don't think you need the BUG_SUPERVISOR_OLD_BUGTASK_STATUSES, though. <deryck> just self.status not in BUG_SUPERVISOR_BUGTASK_STATUSES and new_status not in BUG_SUPERVISOR_BUGTASK_STATUSES, right? <cjwatson> deryck: I felt that it should be OK to set status away from Triaged <deryck> ah, right <cjwatson> this is just a gut feel <cjwatson> if you disagree I don't feel strongly <deryck> cjwatson, what do people normally transition to from triaged? <cjwatson> but it seemed to me that we might want those to be independently controllable <cjwatson> either in-progress etc. (developer) or something like "oh, hang on, this isn't clear after all" incomplete (not sure) <deryck> right <cjwatson> it just didn't seem as obviously final as wontfix is meant to be <deryck> I agree with you on this. it's just the CONSTANT for WONTFIX that I'm not crazy about. <cjwatson> CONSTANT? <deryck> maybe just `self.status is not BugTaskStatus.WONTFIX` <cjwatson> oh right <cjwatson> if you like, sure <deryck> instead of adding BUG_SUPERVISOR_OLD_BUGTASK_STATUSES <deryck> keep someone from chasing down what that is later :-) <deryck> cjwatson, now one issue that will complicate this... <deryck> cjwatson, if it's in WONTFIX, I think the AJAX widget should not allow selecting any other statuses. and I'm not sure how the js code consults canTransitionToStatus <cjwatson> I think it excludes anything for which that returns False <cjwatson> browser/bugtask.py:status_widget_items <deryck> yes, good spot. <deryck> cjwatson, so when you're ready, I'll happily review this for you. :-) many thanks for the fix! jml -- are you ok with this? Or further discussion is required? <jml> not from me. <deryck> excellent. <cjwatson> I'll need to set up ec2test, so ... see you in a bit ;-) <deryck> ok :-) * salgado changed the topic of #launchpad-reviews to: on call: jelmer || reviewing: wgrant || queue: [wgrant,salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jml> salgado, why the 'tee' change? <salgado> jml, to use sudo, I guess <salgado> before that command was executed as root <jml> oh, duh. I should have seen that :) <salgado> not it's as ubuntu <jml> *nod* <jml> salgado, r=me <salgado> thanks jml! * salgado changed the topic of #launchpad-reviews to: on call: jelmer || reviewing: wgrant || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * henninge changed the topic of #launchpad-reviews to: on call: jelmer || reviewing: wgrant || queue: [wgrant, henninge] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * henninge changed the topic of #launchpad-reviews to: on call: jelmer || reviewing: wgrant || queue: [wgrant, henninge(oversized)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * rockstar changed the topic of #launchpad-reviews to: on call: rockstar, jelmer || reviewing: henninge, wgrant || queue: [wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * rockstar changed the topic of #launchpad-reviews to: on call: rockstar || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <rockstar> bac, can I have you do a quick review for me? <bac> rockstar: for you? sure! <rockstar> bac, https://code.edge.launchpad.net/~rockstar/launchpad/code-js-move/+merge/22656 <bac> rockstar: :( <bac> your rename of the code js directory is going to break branches that sinzui and i have in progress <bac> rockstar: so, lp-deps.py is meant to be general purpose, right? each app will have to add themselves to JS_DIRSET? <rockstar> bac, yes. <bac> rockstar: can i get you to talk to sinzui tomorrow before landing this? i want to ensure his changes to discover JS tests will still work. <rockstar> bac, well, we talked about it at the UI meeting, but yes. <bac> we may have to move our js too. <bac> thanks. <rockstar> In fact, I think I'm going to have to move everyone's javascript is moved soon. <rockstar> bac, does that me r=you then? :) <bac> yeah, that makes sense <bac> rockstar: yeah, r=bac but chat with curtis <bac> he won't say not to land it but may need to coordinate some stuff <rockstar> Yeah, I suspected as much. <rockstar> bac, although I bet he's tired of that branch. :) <bac> yes <bac> we all are <cjwatson> should I be expecting to get a mail from johncrosby@example.net about going over a limit on volume of sent e-mail after a full ec2test run? <salgado> I've never got one <cjwatson> seems odd for it to be sending any mail out of the instance <salgado> indeed; on test runs we use the stub mailer <ctrlsoft> I've never had one either; it shouldn't be sending email other than the success/failure email and email to PQM <mwhudson> it certainly shouldn't <wgrant> bac: Can you please re-approve https://code.edge.launchpad.net/~wgrant/launchpad/refactor-slavestatus/+merge/22285? <bac> wgrant: it looks fine <bac> wgrant: do you want me to send it off to ec2 and land now? <wgrant> bac: Ah, I didn't realise it had reopened. Yes please! <wgrant> Thanks. <wgrant> Can someone please also send https://code.edge.launchpad.net/~wgrant/launchpad/move-lots-of-builddmaster-to-queuebuilder/+merge/22009, https://code.edge.launchpad.net/~wgrant/launchpad/bug-540819-fix-builder-list-icons/+merge/22288 and https://code.edge.launchpad.net/~wgrant/launchpad/filter-apt_pkg-deprecationwarnings/+merge/22317 off to EC2? <bac> wgrant: off she goes. let me know if you don't get an email in 4-5 hours <wgrant> bac: Thansks. <bac> wgrant: ask rockstar about those others. part of ocr, you know. <wgrant> bac: Hm, true. rockstar ^^ <jpds> bac: Can you send: https://code.edge.launchpad.net/~jpds/launchpad/fix_361650_model_changes/+merge/20785 ? <jpds> bac: Actually, I should probably let sinzui re-review. #launchpad-reviews 2010-04-02 * salgado changed the topic of #launchpad-reviews to: on call: salgado,rockstar || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <salgado> (completely forgot to add my name to the topic, sorry) <salgado> rockstar, you up for reviewing some massive removals? <rockstar> salgado, I'm not actually reviewing, but yes! <salgado> rockstar, oh, no need to worry, then. it's not urgent, but if you'd like I certainly wouldn't mind. :) <salgado> https://code.edge.launchpad.net/~salgado/launchpad/remove-logintoken-unused-code/+merge/22719 <jpds> sinzui: Hey, did you have time to look at my c_d_m model branch? <sinzui> jpds, yes <jpds> sinzui: Could we possibly land it now? <sinzui> yes, sorry. I was distracted by my son. I will land it now. I think your changes were great <jpds> sinzui: No worries, thanks! #launchpad-reviews 2010-04-03 <wgrant> sinzui: Up for reviewing a branch that reverts those sampledata changes? * wgrant changed the topic of #launchpad-reviews to: on call: salgado,rockstar || reviewing: - || queue: [wgrant(revert-soyuz-sample-data-changes),wgrant(bug-538844-master-side)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * wgrant changed the topic of #launchpad-reviews to: on call: salgado,rockstar || reviewing: - || queue: [wgrant(ttbb-slavestatus-testfix),wgrant(revert-soyuz-sample-data-changes),wgrant(bug-538844-master-side)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews #launchpad-reviews 2010-04-05 <stub> https://code.edge.launchpad.net/~stub/launchpad/garbo/+merge/22680 <stub> https://code.edge.launchpad.net/~stub/launchpad/garbo/+merge/22680 <stub> jtv: ^^ that's a loop tuner patch if you are bored. <jtv> stub: oooh, cool. I'm working on an oversized review, but I may just take this one inbetween as a light snack. * salgado changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * salgado changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup),salgado] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> stub: had some lucid bugs, but looking at your branch now. _is_timed_out should be spelled _isTimedOut; and "extra" is not a great name... maybe extra_seconds or extension_seconds or somesuch? <jtv> (Though definitely nice to have that isolated) <jtv> Maybe invert the condition and call it _haveTimeLeft(seconds=0)? <jtv> ah no, that'd be the wrong way around wouldn't it * jtv continues to flip-flop on that issue <stub> :) <jtv> oh stub, there you are! I just posted a comment. <stub> ta. Just eating dinner :) * abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [wgrant(bug-538844-master-side),wgrant(amputate-buildergroup),salgado] || 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: abentley || reviewing: salgado || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> salgado, what proposal do you want me to review? <salgado> abentley, remove-lp-service-openid <abentley> salgado, lp:~salgado/launchpad/remove-lp-services-openid is not proposed for merging. <salgado> I might have forgotten to create the mp <salgado> yeah, looks like I did <salgado> sorry abentley. just proposed: https://code.edge.launchpad.net/~salgado/launchpad/remove-lp-services-openid/+merge/22807 <salgado> it depends on my remove-auth-store branch, which removes canonical-identity-provider <abentley> salgado, cool. I'll have a look in a few minutes when the diff is up. <salgado> thanks abentley <abentley> salgado, 1912 lines of diff!? * salgado checks <salgado> abentley, the diff shown there includes what's in my remove-auth-store branch <salgado> any idea why it didn't take the pre-requisite branch into account when generating the diff? <abentley> salgado, no idea. <abentley> salgado, are both branches up-to-date? <salgado> yes, I think so. let me try merging remove-auth-store again into the remove-lp-services one <abentley> salgado, I believe you've got a criss-cross merge, because you merged devel into each of them. <salgado> could be <salgado> abentley, any way to solve that so that the generated diff is correct? <abentley> salgado, if you merge remove-auth-store into remove-lp-services, it should be much better. <salgado> abentley, just did that <abentley> salgado, hmm. So that brought it down to 943 lines. Is correct now? <salgado> very likely, let me check <salgado> abentley, yes, that's correct. it's just removals, though, as you can see <abentley> salgado, this is usually when I ask about preimplementation calls, but I assume it's been discussed to death. <abentley> salgado, r=me <salgado> thanks abentley * abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * sinzui changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <leonardr> abentley, looking for a quick review of https://code.edge.launchpad.net/~leonardr/launchpad/no-cookie-vary-header/+merge/22812 <abentley> leonardr, looking <adiroiban> abentley: hi, is there anything I need to do for having this branch landed https://code.edge.launchpad.net/~adiroiban/launchpad/bug-548999/+merge/22271 ? * abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: leonardr || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> leonardr, I believe there should be two blank lines, not one, between one heading and the next in xx-service.txt. Otherwise, looks fine. <abentley> adiroiban, it needs to be run through the full test suite. <abentley> adiroiban, some other developers use ec2 to do this, but I don't use ec2. <abentley> adiroiban, for my own branches, I use my local boxes. <adiroiban> abentley: ok. I still have problems running the full test suite on my computer. Meanwhile, I will ask some other LP developer to send the branch to ec2. Thanks! <abentley> adiroiban, no problem. <abentley> sinzui, changing the topic does not get my attention, so I suggest asking me to perform the review instead of just changing the topic. <sinzui> abentley, I agree. I was told to get my children from school so I did not complete my task * abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> sinzui, looking now. <leonardr> abentley: actually i'm pretty sure you don't have to put 2 blank lines anymore, now that we changed the format from =foo= to foo\n=== <abentley> leonardr, the style guide uses "foo\n===" syntax, and says "Two blank lines are used to separate the start of a new section (a header). " <leonardr> ok <leonardr> abentley: i was going to write a launchpadlib branch with another test, and then i realized i can just put a launchpadlib test in launchpad now <leonardr> so i'll give you an incremental diff in a bit <abentley> sinzui, I think that "an unlisted package" may not convey "not packaged". <sinzui> abentley, I should hope NOT <sinzui> abentley, it is not related to the second button. It means the user has to find a package himself using +ubuntupkg * sinzui think the UI review will be equally scathing * sinzui needs to think how to say ":let me pick the ubuntu package myself" <abentley> sinzui, I see. This is the page that is affected by the "not packaged" indicator, not the page where users select that? <sinzui> The not packages button does not use the radio button. It is a simple button that means stop showing me the form <sinzui> abentley, the problem is that this simple button will not fit in the the portlet. so we discussed moving the link to +ubuntupkg into the vocabulary. That was easy, but the intent is clearly ambiguous now. <abentley> sinzui, I am confused about why you are showing me a version with and without a selection. <abentley> s/selection/suggestion <abentley> sinzui, would it reasonable to present "not packaged in ubuntu" as a radio button? <abentley> sinzui, in lib/lp/registry/interfaces/product.py, I think you mean "project", not "projected", in "The projected can be rechecked after enough time " <abentley> sinzui, I don't know what to do about this, but it seems odd that if I select "This is not packaged in Ubuntu", in a year it will prompt me to specify what package it provides in Jaunty. <abentley> sinzui, ping me when you want to proceed. * abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> sinzui: could you give a UI once over to https://code.edge.launchpad.net/~bac/launchpad/bug-524302/+merge/22180 <bac> sinzui: i just pasted a screenshot into the last comment * sinzui is already doing that <sidnei> i have a large branch needing review here, but im on holiday today. if anyone wants to take a peek, feel free, but no rush: https://code.edge.launchpad.net/~sidnei/lazr-js/yui-3.1.0/+merge/22825 <james_w> abentley: thanks for the review. https://code.edge.launchpad.net/~james-w/launchpad/register-code-import/+merge/22668 is updated if you could have another look. <rockstar> james_w, I suspect abentley has EOD'd for the day. :) <james_w> rockstar: me too, but I thought it was worth a try as the topic hadn't changed <james_w> but it's not important given that I'm not really here <rockstar> sidnei, are you around? <sidnei> rockstar, just happen to be <rockstar> sidnei, so, I'm confused a bit by your branch. Why are we renaming classes, and what are all the other changes as well? <rockstar> It LOOKS like it's YUI 3.1 stuff. <sidnei> rockstar, yui 3.1.0 changed the prefix generated by ClassNameManager from yui- to yui3- <rockstar> sidnei, ah, okay. So is this the branch that will make lazr-js support yui 3.1? <sidnei> rockstar, correct <sidnei> rockstar, the other changes are two kinds: 1) removing wait/resume where tests were failing, seems like it is not needed anymore in those cases and 2) double checking for null nodes <rockstar> sidnei, okay. <rockstar> sidnei, that helps. I'll look over this in a bit, and you should have a review for you in the morning. <sidnei> rockstar, awesome. thanks! <sidnei> rockstar, if you get a chance, try the examples too. i noticed one or two misbehaving but didn't dig much yet. probably trivial to fix. the tests pass though. <rockstar> sidnei, okay. Well, if they don't work, we probably need to figure out why before we land this. <sidnei> rockstar, one of them was the inline editor, when clicking on edit the static text isn't hidden. likely a css issue. #launchpad-reviews 2010-04-06 * thumper changed the topic of #launchpad-reviews to: on call: || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <stub> henninge: https://code.edge.launchpad.net/~stub/launchpad/memcache/+merge/22849 <stub> henninge: Security issue that needs to land before the edge update (about 55 mins?) <wgrant> stub: The branch isn't in stable yet. <wgrant> it's fine. <henninge> stub: looking <wgrant> (and probably won't be until some hours after edge updates tonight) <henninge> stub: Shouldn't that comment be an XXX that links to a bug number where the issue is tracked? <stub> Its not a bug though. <stub> Actually, I just realized I need to turn off caching entirely to not lose functionality, as the tooltips will continue to display old summaries when bugs are changed. <stub> I could put a lower timeout, but that is getting really pointless. <henninge> stub: unfortunately I am not at all familiar with caching issues ... <henninge> stub: but this still sounds like a quick fix to me that needs further investigation to find a proper solution and we should track that in a bug in order not to forget it. <stub> I'm not sure what the bug is. Marked up test is uncachable due to Launchpad features. We either put up with it, or change Launchpad features. Its a discussion. <stub> I've removed the cache: stanza entirely, so the diff is just a reversal of some work that landed an hour ago and an additional comment. I can remove the comment if you like, as I think it is throughout Launchpad rather than a bug comment specific issue. <henninge> stub: ok, that sounds reasonable. <stub> henninge: Reasonable enough for an approve vote? <henninge> stub: certainly, I was waiting for the new push ... ;-) <henninge> stub: r=me <stub> Ta :) * henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: sinzui || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jml> henninge, could I persuade you to take a look at cjwatson's branch? <henninge> jml: branch of what? ;-) I don't see anything on activereviews... * jml looks <jml> ahh, he asked deryck specifically <jml> never mind <deryck> jml, henninge -- sorry, I missed he requested the review due to mail filters. Will get it today. <jml> deryck, thanks. * henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * henninge changed the topic of #launchpad-reviews to: on call: henninge || reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * bac changed the topic of #launchpad-reviews to: on call: henninge, bac || reviewing: lunch || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> good morning <bac> hi deryck <deryck> bac, Hi. <bac> deryck: did you coordinate with brian at all on https://code.edge.launchpad.net/~brian-murray/launchpad/fix-date-closed-definition/+merge/22836? <bac> it's really trivial but appears to not have had a pre-imp call (required but overkill) nor manage the bug at all <deryck> bac, not having looked yet, I don't recall talking about that with him, no. looking now.... <bac> deryck: just thought you may want to speak to him about triaging the bug and claiming it before starting work <bac> the fix itself is trivial... <deryck> bac, sure, we can discuss it. He's on my team next month, so it becomes even more important then. :) <bac> great <deryck> bac, I suspect he does sooooo much bug triage currently that he just wanted to drive by fix something and not worry about the rest. :-) <bac> walking on the wild side, he is <deryck> ah, I had never triaged the bug initially either. He was likely waiting on me. <deryck> all fixed up now. <wgrant> bac: Sorry, I was planning to get Soyuz approval, but we have been rather lacking in the Soyuz department for the past several days. <bac> wgrant: yeah, with julian out of pocket i can see that * henninge changed the topic of #launchpad-reviews to: on call: henninge, bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * henninge changed the topic of #launchpad-reviews to: on call: henninge, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * james_w changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, - || queue: [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <james_w> henninge, bac: https://code.edge.launchpad.net/~james-w/launchpad/code-import-request/+merge/22727 when you get a few minutes please <bac> james_w: will do <james_w> Merci <sinzui> bac: can you look at http://people.canonical.com/~curtis/package-suggestion-portlet.png and think about how to fix the UI? I am tempted to revert the a button and link for packaging links, and moving the Not Packaged button somewhere else in the portlet <james_w> sinzui: heh, looks like you've already fixed the bug I just filed :-) <leonardr> abentley, can you take another look at https://code.edge.launchpad.net/~leonardr/launchpad/no-cookie-vary-header/+merge/22812 ? <abentley> leonardr, sure. <leonardr> gary, i'd like your opinion on the Vary:Authorization issue as well <leonardr> abentley, give it a minute to regen the diff <sinzui> james_w, which bug? <james_w> sinzui: filed like 2 minutes ago, that "[ Link to this Ubuntu package] or (+) Link to Ubuntu Package" confused me <gary_poster> leonardr: ack will look <sinzui> oh, damn, this UI is going to kill me since the current one really confused abentley <sinzui> bac: james_w maybe we need a list of options with a single button. Two of the options are "another unlisted package" and "this project is not packaged in Ubuntu" <james_w> sinzui: that would work for me. I'm also ok with your mockups there. <abentley> sinzui, what happened to you yesterday? You disappeared while I was asking review questions. <sinzui> henninge, you could not see anything in devel because sample data was wacky. It will be fixed soon. That is why I create a screen shot...you do not want to see my mark harness changes to create sane spph <sinzui> abentley, you assumed you needed to choose "an unlisted package" to use the "This is Not Packaged in Ubuntu" button. <abentley> sinzui, I assumed that was meant to be the constant item between the two screenshots. <abentley> sinzui, I asked you a bunch of questions yesterday that you did not reply to. One was "would it reasonable to present "not packaged in ubuntu" as a radio button?" <sinzui> It is. the option means "select another package not in this list" and maybe that is the wording I should have used <gary_poster> leonardr: xx-service.txt looks like it would fail now. Your code says self.response.setHeader('Vary', 'Accept') but your test says <gary_poster> 83 + >>> response = webservice.get( <gary_poster> 84 + ... 'http://bugs.launchpad.dev/api/devel/') <gary_poster> 85 + >>> print response.getheader('Vary') <gary_poster> 86 + Authorization, Accept <leonardr> gary: thanks, don't know how i missed that <sinzui> abentley, I think that is the best solution. I need to find new wording for the button. <abentley> sinzui, Yeah, but the submit buttons were also constant. <gary_poster> cool <leonardr> maybe i spent all my time writing that comment and didn't rerun the test <bac> sinzui: i don't think "an unlisted package" makes sense, especially in the "portlet without suggestion" example <sinzui> bac: "select a package using the package finder?" <bac> yes, better <bac> perhaps when there are suggestions "select some other package using the package finder" and what you wrote for the 'without suggestions' case * bac changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, wgrant || queue: [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * sinzui changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, wgrant || queue: [james_w, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <sinzui> bac: henninge: if you have time, can you review https://code.launchpad.net/~sinzui/launchpad/moderate-empty-messages/+merge/22870 <henninge> bac: you had agreed to do james' branch, right? <bac> henninge: no, i was speaking for the both of us. if you are free it is next up. <henninge> bac: ok, I'll take that first, then. <bac> thanks henninge. how do you like OCR on tuesdays? much better, right? <henninge> bac: yeah, a lot better ... ;-) <henninge> although, to me this Tuesday is like a Monday ... ;) <bac> ah, right, ye of the four day weekend... <henninge> ;-) <henninge> over much too quick ... * henninge changed the topic of #launchpad-reviews to: henninge, bac || reviewing: james_w, wgrant || queue: [sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <henninge> james_w: did you have a pre-implementation chat/call with someone from the code team about requestImport? (more recent than the Karmic UDS) <james_w> I did not <henninge> james_w: Especially when adding stuff to the API, I think it's better to be sure that you and them are still on the same page. <henninge> james_w: So, maybe simplest would be to ask somebody from that team to review the branch, then. OK? <james_w> sure <henninge> thanks * henninge changed the topic of #launchpad-reviews to: henninge, bac || reviewing: sinzui, wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> leonardr, I don't know if you're still wanting my re-review. I was waiting for the test fix. <leonardr> abentley: should be pushed, let me check <abentley> leonardr, maybe my page was stale. <leonardr> abentley: yes, it's rev 10623 <abentley> leonardr, looks fine. r=me <leonardr> great * henninge changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, wgrant || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <henninge> sinzui: review done. * bac changed the topic of #launchpad-reviews to: henninge, bac || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * bac high-fives henning...empty queue <henninge> yeah, cool! ;) * sidnei wonders where rockstar's review stands <rockstar> sidnei, it's a big review. :) I'm working on it though. <sidnei> cool, thanks! <rockstar> sidnei, so, maybe I should ask you: could you please write up a doc or email or something detailed the changes necessary for yui 3.1? <sidnei> rockstar, i don't know all the changes necessary yet. so far only lazr-js specific, and those were mostly css and making the tests pass, which only involved changing tests themselves. <rockstar> sidnei, okay. Could we you maybe start a wiki page on the dev.launchpad.net about it then, so we can add to it? <sidnei> rockstar, sounds like a plan. <rockstar> sidnei, basically, I want a doc that shows us a clear migration plan of the things that definitely need to change. * henninge changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <sidnei> rockstar, https://wiki.canonical.com/Rhinos/YUI/MigrationIssues <rockstar> sidnei, great. Can you send that to the Rhinos list? <sidnei> rockstar, sure. <rockstar> bac, can I hop in your queue in about 15? <bac> rockstar: sure <rockstar> bac, it'll be a bit. Dealing with conflict hell currently. <rockstar> bac, so, here's the little one: https://code.edge.launchpad.net/~rockstar/launchpad/branch-bug-display/+merge/22887 <rockstar> bac, the other one I'm still trying to sort out dependent branch on. <bac> ok * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [rockstar1, rockstar2] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <leonardr> gary, ec2 turned up some very minor test failures on https://code.edge.launchpad.net/~leonardr/launchpad/no-cookie-vary-header . can you review my recent changes? <leonardr> i also had to do a launchpadlib branch, which is here: https://code.edge.launchpad.net/~leonardr/launchpadlib/test-fix/+merge/22905 <leonardr> abentely, if you're still around, you could take care of this instead <leonardr> abentley <salgado> bac, care to review https://code.launchpad.net/~salgado/launchpad/bug-556594/+merge/22906 for me? <bac> salgado: sure * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: salgado || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> leonardr, r=me <leonardr> great <gary_poster> leonardr: on call. do you still need me? <leonardr> gary: no <gary_poster> ok cool * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <rockstar> bac, here's the other one to review: https://code.edge.launchpad.net/~rockstar/launchpad/ihasrecipes/+merge/22890 <salgado> thanks for the review, brad. (and I did that change you suggested) * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: rockstar || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <EdwinGrubbs> bac: I have a tiny branch that will be ready in a couple of minutes. <bac> EdwinGrubbs: ok * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * bac changed the topic of #launchpad-reviews to: On call: bac || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <salgado> bac, if you have a few more minutes, I've updated that m-p you just reviewed with the actual fix for the bug <bac> salgado: ok <EdwinGrubbs> bac: here it is: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-552619-suggesting-removed-packages/+merge/22912 <bac> EdwinGrubbs: i'm way ahead of you. :) * bac 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 #launchpad-reviews 2010-04-07 <gary_poster> I need a testfix review <gary_poster> http://pastebin.ubuntu.com/410297/ <gary_poster> The problem was that buildbot is the only thing that runs ``python2.5 -t test_on_merge.py -vv`` <gary_poster> ec2 test does it differently (not sure how) so it was missed. <gary_poster> I'd prefer to change buildbot to simply run ``test_on_merge.py -vv`` at which point I can remove the nasty os.execv <gary_poster> but this lets us move on for now <gary_poster> mwhudson: ping? <mwhudson> gary_poster: looking <mwhudson> gary_poster: that's horrible <gary_poster> yes it is <mwhudson> gary_poster: changing buildbot isn't that hard, really <gary_poster> ...no, but I should be giving my son a bath :-) <mwhudson> at least if there's a losa around <gary_poster> I suppose I could just back my branch out <mwhudson> gary_poster: how should buildbot invoke test_on_merge.py ? <mwhudson> gary_poster: just with ./bin/py ? <gary_poster> mwhudson: that would work. Invoking it with ``test_on_merge.py -vv`` would also work. Then the diff woud be this: <mwhudson> gary_poster: like so https://pastebin.canonical.com/30183/ ? <gary_poster> mwhudson: exackle. <mwhudson> gary_poster: i can get that change landed if you like <mwhudson> it seems better than that pastebin, tbh <gary_poster> mwhudson: testfix would then be http://pastebin.ubuntu.com/410305/ <mwhudson> gary_poster: ah right <gary_poster> mwhudson: I'll see if a losa is around. If so, will do it. If not, will take you up on your offer with gratitude <mwhudson> gary_poster: want to review https://code.edge.launchpad.net/~mwhudson/lpbuildbot/bin-py-test/+merge/22918 ? <mwhudson> it's probably a good idea in any case <gary_poster> reviewing * mwhudson wonders if anyone wants to review https://code.edge.launchpad.net/~mwhudson/launchpad/allow-import-password/+merge/22924 <jtv> mwhudson: I'm on it * StevenK changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> mwhudson: there's no chance that these passwordful URLs will show up in the UI? <mwhudson> jtv: there's every chance <jtv> mwhudson: isn't that, like, potentially bad? <mwhudson> jtv: not really, it's for cases that require guest:guest as the username/password combo <mwhudson> jtv: i guess some words in the new code import form might be a good idea <jtv> mwhudson: the blink tag almost seems like a good idea here, if you get my point... * wgrant banishes jtv. <jtv> hey, I said "almost"! <mwhudson> jtv: in general, the result of the import is going to be publicly available <jtv> mwhudson: that's fine, but one thing that gives a black-hat an even better guess to all of the average user's passwords than their birthday is... one of their passwords. <jtv> Not that bzr users are likely to be average computer users. But OTOH they're likely to manage more accounts than most. <jtv> Another way to do it would be to obscure the password when displaying the URL. <jtv> mwhudson: Actually I'd prefer the latter because it also protects people who enter a login that has commit rights. Whether those people deserve protection is a matter for BOFHs and PFYs, I guess. :-) <jtv> mwhudson: I have to relocate now. I've added my comment to the MP. * jtv changed the topic of #launchpad-reviews to: On call: jtv || reviewing: - || queue: [StevenK] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> On call: jtv || reviewing: StevenK || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <StevenK> jtv: Hi! <jtv> hi StevenK <jtv> Your code looks good, but the MP is a bit thin! Pre-imp? Lint? <StevenK> jtv: Uh? I'm new here, can you expand 'pre-imp' ? <jtv> Ah. "Pre-implementation call." <jtv> We're expected to discuss each change with someoneâwhether it's someone more knowledgeable about the topic, or a peer, or an outsider, or someone less experiencedâand make every effort to make that discussion by voice. * wgrant imagines the content of the call as "AAAAAAAAAAA it's broken it's broken fix it fix it" <jtv> wgrant: the intended solution should come up at least in passing, thank you. <StevenK> jtv: Oh, right. In both MPs I've got up for review, I've spoken to both Julian Edwards and Michael Nelson via Skype about them <jtv> But otherwise, wgrant hit it on the nose. :) <jtv> Good! <jtv> Because as a non-Soyuz person I'm hardly in a position to know whether this change is as sane as it looks. <jtv> And with "lint," I mean the output of "make lint." I'm not assuming you didn't run it, but we normally at least mention it in the MP as a reminder. <jtv> My own branches generally have a final commit around MP time with the description "Lint." Shows how effective that procedure can be. <StevenK> I've been reading through the coding standards, but I have yet to get into the habit of running make lint before submitting the MP <jtv> Then this would be a good start! <jtv> Good *time to* start, that is <StevenK> Well, running it just blew up with a wierd
{'channel': 'launchpad-reviews', 'month': 4}