#launchpad-reviews 2010-03-01 * henninge keeps forgetting about Mondays ... ;) * 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 * intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: lunch, - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <leonardr> intellectronica, can you review https://code.edge.launchpad.net/~leonardr/lazr.restful/mutators-are-not-named-operations ? <intellectronica> leonardr: sure * intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: lunch, leonardr || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: lunch, - || 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, intellectronica || reviewing: -, - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <henninge> oops <bigjools> hi henninge, I have a review if you don't mind - it's 28 lines and r-c <henninge> bigjools: sure <bigjools> thank you sir * henninge changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: bigjools, - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bigjools> https://code.edge.launchpad.net/~julian-edwards/launchpad/archive-url-bug-520520/+merge/20369 <bigjools> henninge: sorry should have pasted that earlier <henninge> bigjools: r=me * henninge changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bigjools> thanks henninge <EdwinGrubbs> intellectronica: can you review https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-525956-unlink-button/+merge/20334 <intellectronica> EdwinGrubbs: ok * intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica || reviewing: -, Edwin || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <EdwinGrubbs> intellectronica: it should also have a UI review, so you might want to do that first. <intellectronica> EdwinGrubbs: oright * abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: -, Edwin, - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <intellectronica> EdwinGrubbs: ui*=me * intellectronica changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: -, -, - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <james_w> hello reviewers, I have a slightly odd request <james_w> https://code.edge.launchpad.net/~james-w/launchpad/fix-getRequestedReviews/+merge/20378 <james_w> I'd appreciate if someone could check that the test fails without the code change and passes with it <james_w> I don't have a working LP dev environment at the moment, but want this bug fixed * james_w changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: -, -, - || queue [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> hi intellectronica, abentley: can i have a review for https://code.edge.launchpad.net/~bac/launchpad/bug-512408/+merge/20379 ? it is targeted to 10.03, so not a RC candidate <intellectronica> henninge, abentley: would you mind taking this one? i'm working on an r-c candidate <abentley> bac, intellectronica, I'll take it. (henninge seems to be missing) <intellectronica> thanks <bac> thx, abentley <abentley> bac, r=me. Hate that &-encoding is done by cgi.escape. Would expect that to be % encoding. <bac> abentley: can you elaborate? <abentley> bac, the python standard library is silly, because cgi.escape has nothing to do with CGI and everything to do with HTML. <bac> abentley: ah, ok, i see your point. <bac> abentley: i guess i've been numbed to that point by years of use <abentley> bac, it's been a while for me, so I was fooled. Actually, there's an equivalent in xml.sax.saxutils, which is a marginally better place for it. * abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: -, james_w, - || queue [] || 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: henninge, intellectronica, abentley || reviewing: -, -, james_w || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> sinzui: do you have time for a UI review of my branch? https://code.edge.launchpad.net/~bac/launchpad/bug-512408/+merge/20379 <sinzui> I do <bac> sinzui: do you need screenshots or do you want to run it yourself? <sinzui> I will run it <bac> sinzui: gah, i already notice i didn't title the choice list properly. <bac> sinzui: s/Registered upstream project/Ubuntu packages/ <sinzui> okay <EdwinGrubbs> abentley: can I get a secondary UI review from you? <abentley> EdwinGrubbs, I don't do UI reviews. <EdwinGrubbs> noodles775: can you do a secondary UI review for me? <EdwinGrubbs> rockstar, mars, sinzui: can you do a secondary UI review for me? <rockstar> EdwinGrubbs, is it urgent? <EdwinGrubbs> rockstar: not at all <rockstar> EdwinGrubbs, I can do it after lunch (~2 hours) <EdwinGrubbs> rockstar: thanks, https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-525956-unlink-button/+merge/20334 <sinzui> EdwinGrubbs: I may get to it. I am looking at bac's branch now <abentley> james_w, sorry, I can't get the branch to run, myself. See [Launchpad-dev] make silently fails building bin/py * abentley changed the topic of #launchpad-reviews to: on call: henninge, intellectronica, abentley || reviewing: -, -, - || queue [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * intellectronica changed the topic of #launchpad-reviews to: on call: henninge, abentley || reviewing: -, - || queue [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <sinzui> bac: ping <bac> hi <sinzui> I think I have done the impossible while testing your branch <sinzui> I did the pmount scenario first. I delete the link to sp pmount... <bac> and it blew up? <sinzui> bac: then I linked applets to lucid pmount, then returned to pmount. I was promoted to link to pmount, so I did. It worked. I seem to have two projects providing lucid pmount. <sinzui> bac: If I merge your work into db-devel, I am sure I would get a db constraint violation <bac> sinzui: yes, i guess you would <bac> sinzui: let me do the merge and see what happens <sinzui> bac:I think the list of candidates should no contain anything that already has a packaging link for the the ubuntu focus series <bac> sinzui: you're right. * sinzui was expecting a NotOne error. maybe that was in db-devel too <sinzui> bac: okay, I do get a <sinzui> NotOneError: one() used with more than one result available <sinzui> when I visit lucid pmunt <sinzui> pmount <sinzui> bac: I think your solution address the most common desire the link to an sp from a project page. We do not handle the case for multiple series. We could offer a multi-step as EdwinGrubbs created for sp-project-series, but maybe we can just offer a links to alternate series from +ubuntupkg and declare victory. <sinzui> s/address/addresses/ <bac> yes, i got a NotOne error when i visit https://launchpad.dev/ubuntu/hoary/+source/pmount after following your scenario <sinzui> Edwin-lunch: ping me when you are available. I have a question in my review. I think we can answer it together quickly <EdwinGrubbs> sinzui: I'm here <sinzui> https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-525956-unlink-button/+merge/20334 <sinzui> ^ your thoughts on using your new view on the dsp page? <EdwinGrubbs> sinzui: so you want me to point the DSP page's delete button at the unlink template? That will be easy enough, and I can just hold onto the referring url to use for next_url. The DSP puts the remove button to the left of the edit button, but I prefer it on the right like I did for the SP page. <EdwinGrubbs> sinzui: I'll add a variable to hide the edit/remove links on the +packaging page. <sinzui> EdwinGrubbs: I agree with the position. If the DSP really is easy then do it. Thanks for hiding the actions on the listing. <sinzui> bac: EdwinGrubbs: can either of you review this now: https://code.edge.launchpad.net/~sinzui/launchpad/product-release-file-api/+merge/20398 <bac> sinzui: i will <bac> sinzui: when was this bug introduced? <sinzui> I think it was July of last year when we added download counters <bac> gah <bac> sinzui: i have done a review with 'approved' but left it in 'needs review' so you can get an RC <sinzui> bac: June actually <sinzui> fab <sinzui> thumper: I would like your RC for https://code.edge.launchpad.net/~sinzui/launchpad/product-release-file-api/+merge/20398 . The issue actually dates from June of 2009 <thumper> june? <sinzui> The defect was introduced when download counters were added. The code for showing a file was refactored into a link formatter <thumper> sinzui: done <sinzui> fab <sinzui> bac: there was some non-sense with my review of your mp. My code and UI reviews are both approve <bac> non sense? <bac> ah, yes. i think ec2 might puke on that review type <bac> we'll see later in the week <leonardr> gary: at least ready for a review of https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-wadl/+merge/20405 <leonardr> there were some minor changes after you looked at it <leonardr> i'm starting the ec2 test now <gary_poster> leonardr: looking <gary_poster> diff not there yet :-) <leonardr> gary, it's there now <gary_poster> yes looking thanks * abentley changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <gary_poster> leonardr: approved. We should link the branch to the bug and then ask thumper to give a release-critical review so you can land it tomorrow morning on db-stable. <leonardr> gary, do you have the bug handy? <gary_poster> leonardr: no, sorry, I thought you had one, and that it was on the janban board. Perhaps there is not one? <gary_poster> kanban <leonardr> i put it on the kanban but i don't know of any bug--just emails <gary_poster> leonardr: ok fair enough, nm <gary_poster> leonardr: I'll make a bug. :-/ <leonardr> all right <leonardr> thumper, can you give https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-wadl/+merge/20405 a release-critical review? <thumper> aye <thumper> done <gary_poster> leonardr: made bug and linked branch already: https://bugs.edge.launchpad.net/launchpad-foundations/+bug/530354 <mup> Bug #530354: wadl generation is broken after multiversion code has landed <Launchpad Foundations:In Progress by leonardr> <https://launchpad.net/bugs/530354> <gary_poster> thank you thumper <leonardr> gary, great <leonardr> yes, thanks thumper <leonardr> gary: i'll come back in a few hours to see if the ec2 test passed <gary_poster> leonardr: thanks <james_w> https://code.edge.launchpad.net/~james-w/launchpad/fix-getRequestedReviews/+merge/20378 should now be good to go if anyone is willing to take it <james_w> on call: - || reviewing: - || queue [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * james_w changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [james_w] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <james_w> actually, the test isn't quite testing the right thing yet #launchpad-reviews 2010-03-02 <james_w> right, it's confirmed working now. Please to be reviewing https://code.edge.launchpad.net/~james-w/launchpad/fix-getRequestedReviews/+merge/20378 <james_w> also, https://code.edge.launchpad.net/~james-w/launchpad/sync-source-negative-versions/+merge/16861 has been approved for 8 weeks and hasn't landed yet. I think it's been hit by ec2 swallowing mails a couple of times. It would be great if someone could submit again. <james_w> thanks <leonardr> thumper: i need another release-critical review of https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-wadl/+merge/20405 as there were some ec2 test failures * thumper looks <thumper> leonardr: done <thumper> leonardr: Approved revision: 10427 (in case you were wondering) <leonardr> cool * al-maisan changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jpds> bigjools: Morning, lp:~jpds/launchpad/timestamp-archivepublisher-run should be good to land. <bigjools> jpds: it's landing right now :) * jtv changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jpds> bigjools: Woo! :) <jtv> noodles775, you interested in reviewing my branch for the local soyuz setup scripts? https://code.launchpad.net/~jtv/launchpad/bug-527170/+merge/20440 <noodles775> jtv: sure! <jtv> noodles775: great, thanks! <jtv> noodles775: I just realized I haven't gotten around to moving the instructional output all the way to the end since I added GPG key registration... mind if I do that now? <noodles775> jtv: go for it, just let me know when it's ready and i'll merge. <jtv> noodles775: it's pushed, sorry for the delay <jtv> Just moved a "print" down a few lines * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>, jtv] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> gmb: noodles775 is reviewing mine <gmb> jtv: Cool, thanks for letting me know. <noodles775> Thanks jtv * jtv changed the topic of #launchpad-reviews to: gmb || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> /topic: on call: gmb || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> gah <gmb> Heh. * jtv changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> ahhhh <gmb> jtv: It's good to know that I'm not the only one that struggles with /topic. <jtv> it's not us, it's /topic * adiroiban changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>,adiroiban(bug-512307)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * adiroiban changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>,adiroiban(bug-512307),adiroiban(bug-359180-take-2)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <gmb> james_w: Does your branch still need reviewing? I don't see an MP for it in the +activereviews queue <gmb> In the meantime... <james_w> gmb: it should still be there <james_w> https://code.edge.launchpad.net/~james-w/launchpad/fix-getRequestedReviews/+merge/20378 <james_w> and remember https://code.edge.launchpad.net/~james-w/launchpad/sync-source-negative-versions/+merge/16861 ? :-) <james_w> I think that's been hit by ec2 not sending mails a couple of times <gmb> Argh sodbollocks. <gmb> james_w: Right, I'll run that one again but stay connected to make sure we find out what's going on. <james_w> thanks <gmb> Man, LP's slow today... <adeuring> gmb: fancy to review a small rbanch (ca 10 lines interface changes; 100 lines tests): https://code.edge.launchpad.net/~adeuring/launchpad/api-export-bug-linked-branches/+merge/20444 ? <gmb> adeuring: Sure, let me just finish james_w's branch. Won't be a sec. <adeuring> gmb: thanks! <al-maisan> gmb: there's another branch in the queue as well :) <gmb> james_w: r=me <james_w> thanks gmb <james_w> would you submit that one too please? <gmb> al-maisan: I'm in the process of sorting out who's here and who's not and who's just left things lying in the queue. Good to know you're not one of the latter. I'll take a look at yours now. <gmb> james_w: Sure. <james_w> thanks <gmb> adeuring: Please put your branch in the queue; first come first served and all that... <al-maisan> gmb: thanks! <adeuring> gmb: sure * adeuring changed the topic of #launchpad-reviews to: on call: gmb || reviewing: - || queue [james_w, al-maisan<http://tinyurl.com/y8gmlcd>,adiroiban(bug-512307),adiroiban(bug-359180-take-2), adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: al-maisan || queue [adiroiban(bug-512307),adiroiban(bug-359180-take-2), adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <gmb> al-maisan: Is this review for your spurious-tests branch? tinyurl isn't resolving for me atm... <al-maisan> gmb: yes <al-maisan> https://code.launchpad.net/~al-maisan/launchpad/spurious-tests-525329/+merge/20233 <gmb> Cool, thanks. <al-maisan> gmb: not sure whether this matters but lp:~al-maisan/launchpad/spurious-tests-525329 was branched off of devel and not db-devel <gmb> al-maisan: I think you're in luck. The diff's only 123 lines, so I guess the devel revisions in your branch have already been merged into db-devel. <al-maisan> hmm :) <gmb> al-maisan: r=me. <al-maisan> gmb: thanks! <gmb> np * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: adiroiban || queue [,adiroiban(bug-359180-take-2), adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <noodles775> jtv: what would be the problem with just adding amd64 as an arch always? the user still needs to target a specific series? <jtv> noodles775: dunno frankly; I figured the safe thing to do was to stay faithful to what we already had. <noodles775> jtv: also, do you think it should be possible to run this twice? (ie. the first time I didn't set my email, so I need to make schema before re-running right?) <jtv> noodles775: right now you'll have to "make schema" between runs. <jtv> wgrant: is there any conceivable reason not to add amd64 support when setting up the sampledata for soyuz? <wgrant> jtv: No. <jtv> noodles775: there's your answer. No reason. Fixing. <noodles775> jtv: great. <jtv> noodles775: testing the fix. <noodles775> jtv: I wonder if in the future we should separate out utilities/soyuz-add-ppa-user.py? <noodles775> (ie. it could be run multiple times). <jtv> noodles775: we have that option. But I think we'll get a lot of documentation benefit out of sticking with ppa-user for now. <noodles775> Right. <noodles775> Wow... automatic addition of my GPG key :) <jtv> yup <jtv> noodles775: It was hard, but I hope worth it. I just pushed the version that always includes amd64 support. <noodles775> jtv: it seems to have added a third key, in addition to the two identified by my email? <jtv> noodles775: the bogus one is for signing the Code of Conduct. Please don't make me rework that to use the proper key (and prompt you for a passphrase) <noodles775> jtv: ah, no worries :) <jtv> noodles775: but glad to hear that it picked up both keys for someone who had two. :-) <noodles775> jtv: and none of them are found in zeca when clicking on the key id? <noodles775> jtv: actually, if I modify the final url param to op=get it finds it. <jtv> noodles775: yup <jtv> that would've happened with a manual upload as well, app'ly <noodles775> but not op=index.... ok, but it's there :) <noodles775> jtv: Another thought, it'd only be an extra 1 line to create a default PPA for the user in create_ppa_user? (you could just use factory.makeArchive(owner=person)) <jtv> noodles775: I thought that'd be a nice next feature, but I think you just saved me the research. <noodles775> jtv: great! * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: adiroiban || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> noodles775: should I add virtualized=False? <noodles775> jtv: yes, definitely. <jtv> noodles775: running tests... * noodles775 tries uploading a package with the new setup. <noodles775> jtv: oh, and an extra line to add the external dependencies? <jtv> noodles775: here we go, feature creep! <noodles775> lol <jtv> noodles775: is that something you can rattle off off the top of your head? Because if not, I'd prefer to leave it documented as a manual step. And bear in mind that there's the matter of distroseries choice to be considered. <gmb> james_w: I've seen the same error twice now running your sync-source branch: http://pastebin.ubuntu.com/386940/ <gmb> james_w: I'll run it again, but this may need some debugging. <james_w> gmb: I'm not sure I have any clue what's going on there <gmb> james_w: Well, look on the bright side, you're not alone. <james_w> :-) <jtv> noodles775: object lesson in feature creep... I have to merge devel to get the "virtualized" parameter. Just done that, but it introduces an import policy violation. Now you see why I'm hell-bent on restricting scope sometimes. <noodles775> jtv: sure, in which case, just leave it without the PPA creation. I'd be happy to contribute to it later :) <james_w> gmb: you cwd seems to be processapportblobjob-api-bug-513191? Would that have any impact? <noodles775> jtv: r=me <jtv> noodles775: thanks... I'll see if I can resolve the import violation now though; I prefer that over multiple uncommits. :-) <gmb> james_w: It shouldn't; the error's occurring on the ec2 box. My cwd on my machine shouldn't have an impact on that because I tell EC2 which bzr branch to grab. <james_w> right <gmb> james_w: But it's a good idea. If it fails again I'll try branching and running fromthat branch. <james_w> is the paging chopping stuff out? <gmb> james_w: Not AFAICT. <james_w> it seems to be running tests, then stops in the middle of outputting a line <james_w> then appears to be running make clean <gmb> Hmm. Maybe that's bad copy-paste on my part. <gmb> FTR, your other branch is running fine. <james_w> good <noodles775> jtv: ok, well in terms of the external dependencies, it should be as simple as person.archive.external_dependencies = ".." <noodles775> jtv: just if you wanted to try it. <noodles775> (assuming you're currently logged in as admin, that is). <jtv> noodles775: I don't know what that'll do in terms of taking people's testing choices away though. <noodles775> jtv: up to you. I'd think of it as a sensible default that they can always edit if they like. <jtv> noodles775: true * gmb grabs some lunch * gmb changed the topic of #launchpad-reviews to: on call: gmb || reviewing: adiroiban || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * adiroiban changed the topic of #launchpad-reviews to: on call: gmb || reviewing: adiroiban || queue [adeuring,adiroiban(bug-525325)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <jtv> noodles775: you have an ancient lock on the UsingSoyuzLocally page <noodles775> jtv: sorry, should be gone now. <jtv> so it is. * bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: adiroiban, adeuring || queue [adiroiban(bug-525325)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> good morning gmb <gmb> Hi bac <bac> busy today? <jtv> noodles775: all done... a few more paragraphs to scratch from the manual, er, manual. <gmb> bac: Not really; just thrashing through an 800-liner at the moment. <wgrant> jtv: Excellent. <jtv> wgrant: it's in EC2 now. Once it lands, I've got some wiki editing to do. :-) <wgrant> Very good. <jtv> but now, time to enjoy the night. See you tomorrow! <wgrant> See you. <gmb> adiroiban: Can you point me at a page on launchpad.dev where I can see the results of https://code.edge.launchpad.net/~adiroiban/launchpad/bug-359180-take-2/+merge/20122? <adiroiban> gmb: no. what results? <gmb> adiroiban: You've made JS changes; I'd like to see the effects of that in the UI. <adiroiban> gmb: hm... they are just keybindings <gmb> adiroiban: Yes, but I'd still like to actually try them out for myself. <gmb> adiroiban: It's an 802-line diff, so I'd like to see exactly what it's done. <gmb> Actually, that sounds excessive for "just keybindings" <adiroiban> gmb: sure :) <gmb> adiroiban: Is the diff on that mp sane? <adiroiban> gmb: yes. <gmb> Okay. <adiroiban> https://code.edge.launchpad.net/~adiroiban/launchpad/bug-359180/+merge/16422 <adiroiban> Here was the original MP with an extensive QA <adiroiban> the code from this MP was merged into edge...but since there were some problems on edge, it was removed prior to 10.01 release <adiroiban> gmb: so the page https://translations.launchpad.dev/ubuntu/hoary/+source/evolution/+pots/evolution-2.2/es/+translate?start=0 <gmb> adiroiban: Okay, so I'm confused now; is your new branch a resubmission of the old branch? <adiroiban> gmb: yes. for some of the code. <adiroiban> so the initial branch was landed on edge <adiroiban> and then we got some feedback <gmb> adiroiban: Okay. So, I don't think I need to re-review the old code. I just need to see the changes that you've made to that old code, correct? <adiroiban> we discovered some problems with the keybings, and since it was near the release jtv had removed the branch <adiroiban> from edge <adiroiban> gmb: I don't know what is the process for reviewing branches that were pulled out of edge <gmb> adiroiban: Can you paste a diff between the old branch and the new one into the merge proposal please? If the old branch was approved I don't need to re-approve it. I just need to approve the changes that you've made to it to fix the problems that caused it to be pulled in the first place. <adiroiban> gmb: the latest approved revision in this branch was 10168 <adiroiban> but diff -r 10168 is just to noisy <gmb> adiroiban: Okay, I'll work out a way to get a sane diff. <gmb> Thanks. <bac> hi abel <bac> er, adeuring <adeuring> hi bac <bac> adeuring: could you update your branch with devel and repush? <adeuring> bac: sure. is there again a merge conflict? <bac> adeuring: i've merged yours in and got a lot more changes than i expect... i'm a bit baffled. no conflicts. <adeuring> odd... let me see <adeuring> bac: i merged devel again; locally, the diff against devel looks exactly at the one here: https://code.edge.launchpad.net/~adeuring/launchpad/api-export-bug-linked-branches/+merge/20444 <bac> ok <abentley> gmb, bac: Could you please review https://code.edge.launchpad.net/~abentley/launchpad/no-original-to/+merge/20451 <bac> abentley: yes <abentley> bac, thanks. <bac> adeuring: found the problem. PEBCAK <adeuring> bac: ;) <bac> adeuring: when i do a 'make build' on your branch i get errors about the reference to IBugBranch. it works for you? <adeuring> bac: let me check... <bac> adeuring: it looks like bug.py is just missing an import <adeuring> bac: yeah. I am really suprised that I did not get errors running the page test... <bac> adeuring: i'm confused too. the page test errored for me <adeuring> interesting... <adeuring> bac: anyway, I've oushed a new version, containing the missing import statement <adeuring> erm, I've pushed... <leonardr> rockstar, can you give a quick review to https://code.edge.launchpad.net/~leonardr/lazr.restful/tag-web-service-request/+merge/20454 ? it's critical <bac> leonardr: rockstar is not available now. if it is critical you should ask one of the OCRs <leonardr> bac: sure. can i get you or gmb to look at it? <bac> leonardr: i'd be delighted. move yourself to the top of the queue * leonardr changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: adiroiban, adeuring || queue [leonardr, adiroiban(bug-525325)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <salgado> bac, gmb, hi there. I have a RC one, can I jump the queue? <salgado> it's a trivial one, btw <bac> salgado: yes, after leonard <salgado> cool, thanks bac <salgado> https://code.launchpad.net/~salgado/launchpad/bug-530738/+merge/20452 <bac> adeuring: did you test your changes from launchpadlib? if so, please paste a script that you used. <adeuring> bac: no. I did not... * bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: adiroiban, leonard/adeuring || queue [adiroiban(bug-525325)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> adeuring: could you whip up something? <adeuring> bac: sure * adiroiban changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: adiroiban, leonard/adeuring || queue [adiroiban(bug-525325),adiroiban(bug-527728)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> leonardr: that branch looks fine. will you be updating lp to use it now? <leonardr> bac: yes, as part of https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-wadl <leonardr> which is not done yet--i have to fix more test failures <bac> leonardr: is that an RC candidate? <leonardr> bac: i don't know the exact terminology, but thumper gave my last revision of it a release-critical review * bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: adiroiban, salgado/adeuring || queue [adiroiban(bug-525325),adiroiban(bug-527728), abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> leonardr: ok <salgado> thanks for the review, bac. but the bug was introduced a week ago, when I changed the +login page to use OpenID. :) <bac> oh <bac> anyway, good catch <adeuring> bac: sorry, haven't run LP scripts locally since a longer time.. I'm getting a really odd error trying to access the local server: Error fetching XRDS document: <urlopen error (-5, 'No address associated with hostname') ... <bac> adeuring: i'm able to exercise the linked_branches property but unlinkBranch does not work <adeuring> bac: interesting... what is going wrong? <bac> adeuring: it returns a 'bad request' <adeuring> bac: can you show me your script? <bac> adeuring: i'm just poking at it interactively <adeuring> bac: just a hint: there is an oddity with linkes_branches vs. linkBranch() and unlinkBranch(): linked_branches does not return branches themselves, but IBugBranch objects, while (un)linkBranch() expect a branch <adeuring> bac: thanks for the review! <bac> np <bac> abentley: is your branch an RC candidate? <abentley> bac, yes. * bac changed the topic of #launchpad-reviews to: on call: gmb,bac || reviewing: adiroiban, abentley || queue [adiroiban(bug-525325),adiroiban(bug-527728)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bac> abentley: glad i asked. i'll start on it now. <bac> abentley: r=bac. good luck getting your RC <abentley> bac, considering thumper asked for it, I doubt that <abentley> will be hard. <bac> :) * gmb changed the topic of #launchpad-reviews to: on call: bac || reviewing: adiroiban, abentley || queue [adiroiban(bug-525325),adiroiban(bug-527728)] || 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: adiroiban, adiroiban(bug-525325) || queue [adiroiban(bug-527728)] || 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: adiroiban, adiroiban(bug-527728) || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <leonardr> gary: while i hunt down the launchpadlib problem, maybe you can look at https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-wadl/+merge/20405 ? <gary_poster> leonardr: on it. <gary_poster> leonardr: can we get rid of fix_url in that test in the future? Either it is already planned, or you have an idea? <leonardr> gary: ideally we would be able to take a browser-based website request and turn it into a web service client web service request <leonardr> the way we can turn a browser-based website request into a browser-based web service request <gary_poster> leonardr: is it reasonable to ask you to put in a bug for this and reference it, or is this going to be so annoying that it is unlikely to happen? <leonardr> gary: well, it'll happen whenever someone tries to get the url of an object and use it in a web service request <leonardr> i don't know how likely that is, it only happens once now <gary_poster> ...leonardr, would that greatly simplify our JS usage of the webservice? <leonardr> gary: no, because all js usage is already handled. browser-based website request -> browser-based web service request <gary_poster> leonardr: OK. Anyway, I don't want to get in your way. I'm fine with this branch in the short term. If it is reasonable to say that we can get rid of fix_url, I'd prefer a bug for that, but if not, just tell me, move on, and I approve. <leonardr> gary: i think one way or another we can get rid of fix_url <leonardr> but probably by making the test do something more sensible <leonardr> not by making it easier to do what the test does <gary_poster> leonardr: ok. <leonardr> gary, i think the launchpadlib failure may have been introduced by another branch that landed recently, or by me not having an up-to-date launchpad <leonardr> DiscoveryFailure: Error fetching XRDS document: &lt;urlopen error (110, 'Connection timed out') <leonardr> does that look like something that might be caused by the auth split? <gary_poster> leonardr: it looks pretty generic. I don't think the auth split has done anything pertinent in the last few hours. salgado, am I right? <leonardr> i'm updating everything now. i'll try again and if it still doesn't work i'll just put this into ec2test, because i'm pretty sure it's not my error <salgado> leonardr, grep testopenid /etc/hosts <leonardr> salgado: nothing <leonardr> do i need to add something? <salgado> leonardr, either add testopenid.dev to the 936-429-9875 line or run rocketfuel-setup <leonardr> salgado: thanks much <leonardr> gary: launchpadlib error has gone away <leonardr> i'm going to try once more without the new lazr.restful (which shouldn't be necessray) and then start ec2 <gary_poster> yay, leonardr! <leonardr> gary: ok, starting ec2 <leonardr> i'm very confident it will work because i haven't changed launchpad since the time the two tests failed <leonardr> i've just fixed the two tests <gary_poster> makes sense to me. <leonardr> i'm also doing a test of launchpadlib against a running launchpad.dev to make sure the whole thin works <gary_poster> have some lunch. :-) * bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: adiroiban(bug-527728) || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <leonardr> gary: live launchpadlib test works fine <gary_poster> great, leonardr. <bac> hi adiroiban <adiroiban> bac: hi <bac> adiroiban: i'm reviewing your branch for bug 527728. did you happen to test getting component_name from lplib? i'm trying and cannot get it to work. <mup> Bug #527728: Export source package component in API <api> <Launchpad Registry:In Progress by adiroiban> <https://launchpad.net/bugs/527728> <adiroiban> bac: no. I was just testing from a webbrowser <bac> adiroiban: here is what i'm seeing: http://paste.ubuntu.com/387146/ <bac> adiroiban: it's always a good idea to test new changes to the API interactively with launchpadlib to ensure the WADL got generated properly, etc. (in fact reviewers are supposed to ensure it has been done.) <adiroiban> bac: but bin/test -t source-package.txt is OK? <bac> adiroiban: yes <adiroiban> bac: have you tried deleting the old wsdl file ? <bac> not yet <bac> ah, perhaps it is cached <adiroiban> not sure, but I know the makefile does not build it properly <adiroiban> so during devel I have to delete it manualy <adiroiban> and then it is regenerated <bac> adiroiban: i've removed my cache and am trying again <adiroiban> bac: do I need the latest build from launchpadlib, or it is ok to test with karmic version? <bac> karmic is fine <bac> adiroiban: or you can use the one packaged with lp by using 'make iharness' <adiroiban> and how can I tell it to use the lp.dev <bac> login_with('somename', 'dev') <adiroiban> bac: sorry. the name is latest_published_component_name <adiroiban> so instead of sp.component_name use sp.latest_published_component_name <bac> adiroiban: no, it isn't. you have no "exported_as" clause <bac> it now works that i got rid of my cache <bac> In [7]: ev.component_name <bac> Out[7]: 'main' <adiroiban> bac: hm... then something is fishy <bac> adiroiban: did you not push the latest version <adiroiban> on https://launchpad.dev/+apidoc/index.html#source_package I can see the key exported as latest_published_component_name <bac> https://code.edge.launchpad.net/~adiroiban/launchpad/bug-527728/+merge/20456 <bac> the diff at line 8 shows you didn't set an 'exported_as' <bac> also, your test fails b/c the component output is in the wrong place <adiroiban> I have pushed the code. The latest revision should be 10428 <adiroiban> bac: on my local branch the lib/lp/registry/tests/../stories/webservice/xx-source-package.txt has 0 failures <bac> adiroiban: ok, i just grabbed your new version. it does not match what is in the MP or what i grabbed an hour ago <bac> adiroiban: but that's fine -- it explains the mystery <adiroiban> bac: sorry for this mess! <bac> np. glad it is figured out <bac> yay, your tests pass now * bac changed the topic of #launchpad-reviews to: on call: bac || reviewing: <your name here> || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> bac, could you please review https://code.launchpad.net/~abentley/launchpad/branch-scanner-zcml/+merge/20471 ? <bac> abentley: RC? <abentley> bac, no. <bac> abentley: ok, i'll get right on it anyway. just wondered. <leonardr> gary, do i need thumper or someone to approve the changes to https://code.edge.launchpad.net/~leonardr/launchpad/multiversion-wadl/+merge/20405 for release-critical? <gary_poster> leonardr: I don't think so. He's approved it twice. :-) He wants it fixed. <leonardr> ok, cool <thumper> leonardr: yes I do <thumper> leonardr: want it fixed that is <bac> hi rockstar <rockstar> bac, hi <bac> hey rockstar, i notice your JS reorg branch has been languishing/rotting for a long time. any reason you haven't gotten it reviewed? do you want me to review it? <bac> is it still current? <rockstar> bac, which one? <bac> https://code.edge.launchpad.net/~rockstar/launchpad/code-js-reorg/+merge/20170 <rockstar> code-js-reorg ? <rockstar> bac, yes, I have a topic for the reviewers meeting tomorrow that is blocking it. <rockstar> bac, I'm several pipes from there right now. <bac> so, by "yes" you mean "no, don't review it"? <bac> rockstar: please add your topic to the ReviewersMeetingAgenda <rockstar> bac, yes, that's what I mean. I'll get it reviewed as soon as we have a consensus with the rest of the reviewers team (the UI Swat team is already okay with it) <bac> rockstar: if it is contentious or interesting would you present it at the AMEU meeting? <bac> else you'll have to wait another week <rockstar> bac, I plan on presenting it at the AMEU meeting. <bac> excellent <rockstar> (which means I'll miss my bike ride in the morning) <bac> boo <rockstar> bac, you're assuming that I'm not sore from yesterday's ride. :) <bac> what is your temperature cut-off for riding? <bac> isn't it still crazy cold there? <rockstar> bac, it's actually really nice here right now, ~50 degrees or so. <bac> much nicer than us... <rockstar> The lowest I've ridden was -18F. I have good warm riding clothes though. <rockstar> The last few weeks it's been nice during the week and then snows all weekend. <bac> rockstar: i changed the status of that branch to WIP to avoid confusion <bac> er, MP not branch <rockstar> bac, okay, whatever works best for reviewers is good with me. * bac changed the topic of #launchpad-reviews to: on call: - || reviewing: <your name here> || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews #launchpad-reviews 2010-03-03 * allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: ? || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <al-maisan> hello allenap, I have a nice branch for you :) <al-maisan> https://code.edge.launchpad.net/~al-maisan/launchpad/ppa-visibility-514824/+merge/20530 <allenap> al-maisan: Cool, I'll do that now. * allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: al-maisan || queue [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <al-maisan> thanks! <allenap> al-maisan: In xx-distribution-packages.txt, the disabled PPA is created, and it's absence from the PPA list is the test. Can you add a comment along the lines of "The disabled PPA is not shown in the page". Or, better, demonstrate that it is shown, disable it, and demonstrate that it is no longer shown. <al-maisan> allenap: I'll add the comment if you don't mind. Our test suite already takes too much time to run :) <allenap> al-maisan: Okay :) <allenap> al-maisan: Also, ViewSourcePackagePublishingHistory could be changed to just inherit from ViewArchive, with a custom __init__() that calls the superclass with obj.archive. <al-maisan> allenap: good point .. I'll look into that. <allenap> al-maisan: ViewArchive.checkAuthenticated() already checks for user.is_admin, so both ViewSourcePackagePublishingHistory.checkAuthenticated() and ViewSourcePackagePublishingHistory.checkUnauthenticated() both defer entirely to ViewArchive. <al-maisan> allenap: right. <allenap> al-maisan: You don't need to change that though; the composition is quite understandable. <allenap> al-maisan: I have to go and feed children, but I'll be back in <1h. Other than those observations, it all looks good :) <al-maisan> allenap: take care of the children and thanks for the review! <adeuring> allenap: could you please review this mp: https://code.edge.launchpad.net/~adeuring/launchpad/bug-511240-bug-heat-include-duplicate-bugs/+merge/20533 ? a real bargain: only 80 lines * adeuring changed the topic of #launchpad-reviews to: on call: allenap || reviewing: al-maisan || queue [adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bigjools> al-maisan: I have a security fix, can I usurp the queue please :) <bigjools> err allenap, sorry <allenap> adeuring: Sure. <allenap> bigjools: Sure. * allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: al-maisan || queue [bigjools, adeuring] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <allenap> Hey bigjools, where's your mp? <allenap> T'ain't in t'queue. <bigjools> allenap: I'm making a new one, the branch was for devel and the MP was for prod-devel so the diff was screwed <allenap> bigjools: Okeley dokely. * bigjools fights bazaar merging <leonardr> allenap, may i join the queue? (https://code.edge.launchpad.net/~leonardr/lazr.restful/refactor-tag-request-with-version/+merge/20547) <allenap> leonardr: Sure, go for it :) * leonardr changed the topic of #launchpad-reviews to: on call: allenap || reviewing: al-maisan || queue [bigjools, adeuring, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <al-maisan> hello allenap, here are the changes we discussed prior to the lunch break: http://pastebin.ubuntu.com/387623/ <allenap> al-maisan: They look good. Push them and I'll add my r=me to the mp. <al-maisan> allenap: will do, thanks! <al-maisan> changes pushed <danilos> adiroiban, btw, first things first: bug 525325 definitely doesn't belong in LP Foundations; I'd say it's either registry or translations, but that's a very minor point :) <mup> Bug #525325: Export more ILanguage attributes in API <Launchpad Foundations:In Progress by adiroiban> <https://launchpad.net/bugs/525325> <adiroiban> danilos: ok. I asked on lp-dev about the code from services/worlddata and I was told it is part of foundations <danilos> adiroiban, hum, ok; I'd say nobody really knows what is it part of, but LP Translations team mostly maintains it (which is why I never bothered moving browser and other code out of lp.translations) * allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: adeuring || queue [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews * allenap changed the topic of #launchpad-reviews to: on call: allenap || reviewing: - || queue [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> allenap, could you please review https://code.edge.launchpad.net/~abentley/launchpad/qa-ready/+merge/20563 ? <danilos> adiroiban, hey, your branch is looking great, though I have a few questions :) <allenap> abentley: Sure, please add it to the queue. I won't get to it for a couple of hours because I have leonardr's branch to do and some groceries to get (my wife is too ill, still, to do that). <danilos> adiroiban, is there any reason not to use "only visible languages" for the default /languages call? and then provide an extra method that can return a subset of or all languages? * abentley changed the topic of #launchpad-reviews to: on call: allenap || reviewing: - || queue [leonardr, abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> allenap, thanks. <allenap> leonardr: Actually, I'm not going to get to your branch either for an hour or so, due to grocery shopping. If it's urgent you might want to find another reviewer. <allenap> Sorry. <leonardr> allenap, not too urgent <allenap> Cool, I'll do both of those when I'm back. <adiroiban> danilos: the reason was to avoid exporting another `reallyGetAllLanguages` method <adiroiban> other than that, there is no reason. I can do that :) <danilos> adiroiban, well, I'd probably make it something like "getHiddenLanguages" or something; I am not sure myself what's the better pattern, so I'd like to discuss it before we settle on either way <adiroiban> danilos: then we can export only visible languages by default <adiroiban> and have a getAll operation that will get all languages <danilos> adiroiban, right, so how about a getVisibleLanguages as the default and getAllLanguages as another method? <adiroiban> I am not sure about the usage of getHiddenLangauges, but I guess it will always be used togheter with getVisibleLanguages <adiroiban> so getAll langauges can avoid a second call <danilos> adiroiban, though, will we ever have someone using API and not wanting hidden languages? for that matter, will we ever have anyone wanting to get all languages in the first place? <adiroiban> danilos: hidden languages can be used in Ubuntu to find our imported translations that diverge from the `main` language <adiroiban> other than that, I think most users will only care about visible languages <adiroiban> danilos: thus, I think we can just export all visible languages by default <adiroiban> and if someone ask <adiroiban> we can add an operation to export all languages (including hidden one) <adiroiban> in the current lazr.restful implementation the default operation has no name, and it is available via +languages <adiroiban> to get all langauges, you will need to explicitly call an operation +languages?ws.op=getAll <danilos> adiroiban, right, I'd tend to agree <adiroiban> danilos: ok. then I will change the code to export only visible languages <adiroiban> other issues? <danilos> adiroiban, I'm still going through it, if any, it's going to be only minor issues <danilos> adiroiban, stylistic and such <noodles775> Hi allenap, no probs. if you won't get to it, but I'll just pop this one on the queue: https://code.edge.launchpad.net/~michael.nelson/launchpad/522517-multiple-subscriptions-displayed/+merge/20567 * noodles775 changed the topic of #launchpad-reviews to: on call: allenap || reviewing: - || queue [leonardr, abentley, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <bigjools> allenap: replied to your review <bigjools> allenap: the branch page is showing a diff with stuff missing though :/ <bigjools> and now it's there after I pushed a new revision. Weird. * EdwinGrubbs changed the topic of #launchpad-reviews to: on call: allenap, Edwin || reviewing: - || queue [leonardr, abentley, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <EdwinGrubbs> leonardr, abentley, noodles775: do any of you have urgent branches? <abentley> EdwinGrubbs, no. <leonardr> edwingrubbs, no <noodles775> EdwinGrubbs: no. <EdwinGrubbs> abentley: I'll take yours, since it looks like allenap said he would take leonardr's. * EdwinGrubbs changed the topic of #launchpad-reviews to: on call: allenap, Edwin || reviewing: abentley || queue [leonardr, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> EdwinGrubbs, cool. <leonardr> allenap, Edwin, i've got another branch for the queue <leonardr> https://code.edge.launchpad.net/~leonardr/lazr.restful/make-test-class-public/+merge/20574 * leonardr changed the topic of #launchpad-reviews to: on call: allenap, Edwin || reviewing: abentley || queue [leonardr, noodles775, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <EdwinGrubbs> abentley: since I'm not really experienced in this part of the code, did you have a pre-impl call? <allenap> leonardr: I don't think I'll get to that, but I am about to start your first branch. <abentley> EdwinGrubbs, no. <EdwinGrubbs> abentley: how do I go about testing it? I know how to create a mirrored branch, but do I need to run a cronscript to do the mirroring? <abentley> EdwinGrubbs, that's not what we mean by mirror in this case. <abentley> Your mirrors are your local copy of stable and your local copy of db-stable. <EdwinGrubbs> abentley: ok, it's making a lot more sense now. I'll try running it. * allenap changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: abentley || queue [noodles775, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <EdwinGrubbs> abentley: I just want to confirm that this is the intended behavior: <EdwinGrubbs> Mirror file:///home/egrubbs/canonical/lp-branches/db-stable/ is out of date. <EdwinGrubbs> Deployed on staging: False <EdwinGrubbs> abentley: when I updated the local mirror of db-stable, it then said True. <abentley> EdwinGrubbs, that sounds reasonable, if your local copy of db-stable is out of date. <abentley> Updating your mirror of db-stable should not change the status, though. <EdwinGrubbs> abentley: is that because it does graph.is_ancestor() against the local_branch instead of the remote one? <abentley> EdwinGrubbs, I assume it is, but that <abentley> is strange, because I wil have just updated the local branch in that case. <abentley> You weren't running qa-ready against your db-stable branch itself, were you? <EdwinGrubbs> abentley: no. <abentley> That is baffling. <EdwinGrubbs> abentley: according to your cover letter, "It has no effect on <EdwinGrubbs> the branch, only on what revisions are accessible in the branch's repository." <abentley> EdwinGrubbs, correct. <EdwinGrubbs> abentley: so would that explain why graph.is_ancestor() can't find it in local_branch.last_revision()? <abentley> EdwinGrubbs, hold up. <abentley> It appears to be because it does graph.is_ancestor against the local *mirror*, not the local branch. <abentley> Graph traversal is a repository operation. <abentley> So updating the repository of the local mirror should be sufficient. <EdwinGrubbs> abentley: are you going to be able to fix that easily? <abentley> So pull in your mirror of db-stable should have no impact on graph traversal. <abentley> EdwinGrubbs, I suspect that diagnosing the problem is going to be the most effort. <abentley> At this point, I still don't see how the behaviour you're reporting is possible. <abentley> EdwinGrubbs, could you verify that the revno of your db-stable mirror matches the revno of db-stable itself? <EdwinGrubbs> abentley: I had pulled it before, and they are both at rev 9072 now. <abentley> EdwinGrubbs, do you know what revno it was on before you pulled it? <EdwinGrubbs> abentley: this is interesting. If I uncommit the changes to my local db-stable branch and then run qa-ready, "Deployed on staging" is still True. <EdwinGrubbs> abentley: before I pulled it, it was several months old. <danilos> adiroiban, btw, it seems my review didn't get through via email; I've pasted it directly on the MP: only a few small bits left, it's a really great branch! thanks again :) <abentley> EdwinGrubbs, when you uncommitted, you uncommitted back to where it was before? <EdwinGrubbs> abentley: no, I just uncommited till I got to the last revision where stable is merged into db-stable. "bzr missing" shows that the branch I'm running it on has one extra revision. <abentley> EdwinGrubbs, I'm trying to reproduce the issue locally. <leonardr> edwin: another super small branch for the queue <leonardr> https://code.edge.launchpad.net/~leonardr/launchpadlib/multiversion-fix/+merge/20585 * leonardr changed the topic of #launchpad-reviews to: on call: Edwin || reviewing: abentley || queue [noodles775, leonardr, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews <abentley> EdwinGrubbs, I was unable to reproduce the issue locally. With a fresh branch of db-stable that was 50 revisions out of date, it still correctly determined the status of a branch that was deployed this morning. <EdwinGrubbs> abentley: I think that it is finding the revisions in the repository if they already are there, but the update of the repository during the script either doesn't update the repository correctly, or the script is ignoring the new data since the object were opened before that was added. <abentley> EdwinGrubbs, that's why I used a fresh branch that doesn't have those revisions. <EdwinGrubbs> abentley: but isn't that branch using a shared repository? <abentley> EdwinGrubbs, no, I created it as a standalone branch so that I could get an accurate test. <EdwinGrubbs> abentley: hmmm, that's weird <abentley> EdwinGrubbs, it may have used the revisions from the shared repository via the local_branch, so I'm running another test. <abentley> EdwinGrubbs, my second test also doesn't reproduce the problem. I used a fresh copy of db-stable and the branch in question, and neither had the revision where the branch was merged. <EdwinGrubbs> abentley: ok, we'll assume it is a fluke, and wait for it to happen again. r=me <abentley> EdwinGrubbs, thanks. <leonardr> edwin, i'm leaving for the day. feel free to drop my branches from the queue if you find anything confusing <EdwinGrubbs> leonardr: how do I run the tests. <leonardr> EdwinGribbs: which branch? <leonardr> python bootstrap.py; bin/buildout; bin/test should work <EdwinGrubbs> trying it now <sinzui> EdwinGrubbs: ping #launchpad-reviews 2010-03-04 * EdwinGrubbs changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue [noodles775, leonardr] || 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: bdmurray || queue [noodles775] || This
{'channel': 'launchpad-reviews', 'month': 3}