[00:04] mwhudson: I'm still working on it. My python abilities are not that great (I prefer Perl), so it might take me a little while to make the necessary changes [00:05] nhandler: ok [00:05] nhandler: let me know if you need help [00:05] mwhudson: Will do. === abentley1 is now known as abentley [02:43] mwhudson: I figured out how to override the description. Should I override it in both of the two locations that review_type is used? Or only in one of them? Either method will result in the correct string being displayed on the correct page [02:43] nhandler: i'm not completely sure [02:44] mwhudson: Overriding it in both locations would probably be safest. [02:44] nhandler: part of me is saying that the description in the interface should be aimed at the programmer, and _all_ of the uses of it in a form should override it to something appropriate for a human [02:45] nhandler: perhaps more particularly, in the interface the description should describe what the field means in the object [02:46] nhandler: as the description in the interface ends up here: https://edge.launchpad.net/+apidoc/#code_review_vote_reference [02:49] mwhudson: Alright, I'll update the description in the interface and add overrides in the 2 locations that the review_type is used [02:49] nhandler: awesome, thanks [03:56] mwhudson: I think I have a correct patch now. Since I am modifying actual code now, do I need to run some tests on it? Can I do this without having an Amazon EC2 account? [03:57] nhandler: you could probably run the codereview page tests locally; they shouldn't take that long [03:57] nhandler: i'll run all the tests before landing anyway [04:01] mwhudson: Exactly how do I run the test? https://dev.launchpad.net/Testing only shows how to do it in ec2. [04:02] nhandler: ./bin/test -vvct stories/codereview i think [04:02] nhandler: that's a pretty terrible page [04:04] lib/lp/code/stories only contains branches, codeimport, feeds, __init__.py, and webservice. There is no stories/codereview (unless you are talking about a different stories directory) [04:04] hmm! [04:06] nhandler: try code/stories/branches [04:10] Well, I tried actually running the first command. It started to run, but crashed with 'ImportError: No module named Crypto.Cipher [04:13] nhandler: i hope https://dev.launchpad.net/Testing is a little more sensible now [04:13] nhandler: do you have launchpad-developer-dependencies installed? [04:14] mwhudson: Yes. It got installed when I went through the guide for setting up Launchpad [04:14] nhandler: hmm [04:15] nhandler: can you pastebin the output of the trying to run the command? [04:26] mwhudson: http://paste.ubuntu.com/288999/ [04:27] nhandler: hm, that shouldn't happen, but it also shouldn't prevent you from running the other tests [04:28] nhandler: do you have python-crypto installed? [04:29] mwhudson: That fixed the issue I had. But I get: Total: 0 tests, 0 failures, 0 errors in 0.000 seconds. [04:29] Which does not seem right [04:29] nhandler: try ./bin/test -vvct stories/branches [04:30] (this will run more tests than you really need to, but better than none) [04:32] I assume this topic is out of date? === stub changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com [04:33] Trivial and boring review waiting at https://code.edge.launchpad.net/~stub/launchpad/librarian-gc/+merge/13048 if someone can be arsed. [04:35] stub: luckily for you i'm working on a tests in the appserverlayer [04:37] mwhudson: Tests all passed. I submitted a new merge request (https://code.launchpad.net/~nhandler/launchpad/bugfix296469.2/+merge/13103). https://code.launchpad.net/~nhandler/launchpad/bugfix296469/+merge/13034 can be closed. [04:37] And I really appreciate all of the help you have provided. I have learned a lot [04:38] nhandler: spending time to help new developers along is _always_ worth it [04:48] nhandler: you should be able to delete https://code.edge.launchpad.net/~nhandler/launchpad/bugfix296469/+merge/13034 [04:49] mwhudson: I can delete the merge request, but it also deletes the comments. I thought there was a way to change the status so it no longer shows up in the proposed merges list, but is still available online [04:53] nhandler: you can probably set the status to 'rejected' i guess [04:53] i thought there was a way to withdraw it... [04:53] nhandler: in general, you didn't need to make a new merge request though [04:54] nhandler: a comment saying "can you look again" would have been fine [04:55] mwhudson: I can only set the status to 'Needs review', 'Work in Progress', and 'Merged'. The reason for the second merge request was that a second branch was created (due to me not messing around with bzr merge to resolve a divergence) [04:56] nhandler: oh strange [04:56] nhandler: i lose track between what's planned and what's implemented i'm afraid :/ [04:59] mwhudson: Not a problem. The same thing happens to me with some of my other projects. [05:00] nhandler: review sent -- closer, but not quite there yet [05:20] mwhudson: 3rd time is the charm. I just pushed the changes you suggested [05:22] nhandler: awesome [05:25] nhandler: i see you've signed the agreement so i'll see about landing this branch [05:25] * nhandler hugs mwhudson [05:25] You rock mwhudson ! [05:26] nhandler: can you supply a commit message at https://code.edge.launchpad.net/~nhandler/launchpad/bugfix296469.2/+merge/13103/+edit-commit-message ? [05:27] mwhudson: Sure. Should it follow the format on https://dev.launchpad.net/PQMCommitMessages ? [05:28] nhandler: yes, basically [05:29] nhandler: you don't need to worry about the stuff in square brackets (the [r=foo] stuff) [05:29] nhandler: the script i'll be using adds that [05:33] mwhudson: Done [05:37] nhandler: i trimmed it a bit, but thanks! [05:39] mwhudson: So there isn't a need to include the bug number? [05:39] nhandler: no, that page is a little out of date really [05:39] * nhandler loves outdated documentation ;) === jtv is now known as jtv-sprint [09:09] bigjools: Can I grab a tiny review from you? [09:10] wgrant: if it's tiny, yes [09:10] bigjools: I hope it's sufficiently tiny: https://code.edge.launchpad.net/~wgrant/launchpad/bug-433739-debug-archive-urls/+merge/13107 [09:12] yes, it's tiny :) [09:14] wgrant: I'll land it for you [09:15] bigjools: Thankyou. [09:15] * bigjools wishes I could do that without making a local branch === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com [09:16] bigjools: ec2 land! [09:16] I don't use ec2 [09:17] Pfft. [09:18] I think I'd still need a local branch with that anyway [09:20] I wish pqm knew about lp: urls [09:37] bigjools: you don't need a local branch, all you need to do is to send a mail to pqm telling it to merge the branch [09:38] BjornT: true, but not exactly user-friendly :) [09:39] bigjools: another option is to hack pqm-submit not to require a local branch :) [09:39] or put a "land me!" button on the MP [09:42] web UIs are overrated [09:43] let's go 100% API! [09:53] more ctk apps. [09:53] gtk apps, I mean [10:12] you mean qt [10:16] let's not fight over gtk vs qt, and settle for cli instead [10:34] adeuring: can you review this small patch? https://code.edge.launchpad.net/~bjornt/launchpad/jscheck-all-layers/+merge/13113 [10:34] BjornT: sure [10:37] BjornT: r=me [10:37] thanks [10:53] hello adeuring, how are things? [10:53] al-maisan: fine, thanks. Need a review ;) [10:53] sure :) [10:53] https://code.edge.launchpad.net/~al-maisan/launchpad/unembargo-443075/+merge/13114 [11:05] adeuring: re. the branch: the code that send the announcement email is moved to PU.realiseUpload() because that method is invoked in a context that has access to the restricted librarian. This is not the case for the acceptFromCopy() method that thus failed in a production setting. [11:06] al-maisan: thanks for the explanation! [11:06] sorry for not putting that in the cover letter [11:13] adeuring: I will need to break for lunch in approx. 15 minutes but will be back later. [11:14] al-maisan: enjoy lunch! [11:14] :) === ursula is now known as Ursinha [11:43] adeuring: Nice simple review for you :) https://code.edge.launchpad.net/~matthew.revell/launchpad/help-continue-button-bug-406394/+merge/13115 [11:44] mrevell: sure. But let me first finish a review for Muharem === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: al-maisan || queue: [mrevell] || This channel is logged: http://irclogs.ubuntu.com [11:44] of course :) === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: mrevell || queue: [] || This channel is logged: http://irclogs.ubuntu.com === mrevell is now known as mrevell-lunch === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com [12:55] mrevell-lunch: r=me === matsubara-afk is now known as matsubara === mrevell-lunch is now known as mrevell [13:47] thanks adeuring [13:47] mrevell: welcome :) === barry changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com [14:23] adeuring: morning! i am both chr and ocr today, so we'll see how it goes [14:23] barry: morning! [14:25] barry: if you want to check this sort of multitasking right now, I have an MP: https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-class-udev-device-2/+merge/13118 ;) [14:25] adeuring: i'll put it on the list. i have a meeting coming up but i'll take a shot at it === barry changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: -,adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com [14:26] barry: thanks! [15:56] adeuring: Can you review an ec2test-remote.py change please? https://code.edge.launchpad.net/~allenap/launchpad/ec2-test-remote-non-ascii-issue-bug-447247/+merge/13126 [15:56] allenap: sure === adeuring changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: allenap,adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com [16:02] adeuring: I've noticed an issue with it, sorry. It's small, so I'll add paste you an incremental diff. [16:02] allenap: ok, np [16:06] adeuring: Comment added with the diff. Thanks. [16:09] allenap: nice branch! just one question: can we be sure that summary is alway a Unicode instance? Or: Does it alsways have an encode() method? === ursula is now known as Ursinha [16:17] adeuring: I think it will always be a unicode instance, but even str has an encode method so I think it'll be fine. [16:18] allenap: ah, right. so, r=me [16:25] adeuring: Thank you :) [16:25] allenap: welcome :) [16:29] adeuring: review sent === barry changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: allenap,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com [16:29] barry: thanks! === ursula is now known as Ursinha === matsubara is now known as matsubara-lunch === beuno is now known as beuno-lunch === deryck is now known as deryck[lunch] [17:54] rockstar: thanks. i actually had a screenshot in the bug report, but forgot to mention it in the mp. i wish you could attach things to the mp :/ === matsubara-lunch is now known as matsubara === adeuring changed the topic of #launchpad-reviews to: on call: barry || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com === deryck[lunch] is now known as deryck === beuno-lunch is now known as beuno [21:09] barry: time for a very simple review? [21:10] barry, I can has review? [21:10] bac, rockstar put it on the queue. i can get to it around chr duties i think === barry changed the topic of #launchpad-reviews to: on call: barry || reviewing: - || queue: [bac,rockstar] || This channel is logged: http://irclogs.ubuntu.com [21:10] https://code.edge.launchpad.net/~bac/launchpad/bug-435599/+merge/13141 [21:10] barry: it's really more like an rs [21:10] barry, chr AND Orc duties today? Eep. [21:10] rockstar: yep! [21:10] bac: looking [21:11] in fact, i feel almost silly asking [21:11] barry, I'm wondering if that's such a good idea. [21:11] bac: r=me :) [21:11] rockstar: probably not :) [21:11] barry: thanks. [21:11] barry, I'll add it to the reviewer agenda. [21:12] rockstar: interestingly, this is not the first time it's happened to me [21:12] rockstar: i re-arrange my schedule so it doesn't happen === barry changed the topic of #launchpad-reviews to: on call: barry || reviewing: - || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com [21:12] bac, yeah, that's probably a good idea. [21:14] rockstar, bac agreed, though today it's been fairly light [21:16] barry, https://code.edge.launchpad.net/~rockstar/launchpad/cleanup-javascript/+merge/13143 [21:16] barry, no real code changes, just making the existing tests part of a test case with CodeWindmillLayer so that the tests can be run with bin/test [21:17] barry, really I should have landed this branch a while ago. [21:21] rockstar: why no diff on the mp? [21:21] barry, I think a script has exploded. === barry changed the topic of #launchpad-reviews to: on call: barry || reviewing: rockstar || queue: [] || This channel is logged: http://irclogs.ubuntu.com [21:31] rockstar: do you think this will be fixed soon? i'll be around for maybe another hour [21:32] barry, how bout I just give you diff? [21:32] rockstar: that'll work [21:45] barry, http://pastebin.ubuntu.com/289549/ [21:48] rockstar: delete line 149 or 150 [21:49] barry, ah yeah. [21:49] rockstar: delete line 235, 236 or 237 [21:49] rockstar: nice simple branch. everything else looks fine. r=me [21:49] barry, thanks. [21:49] * rockstar was a little whitespace crazy [21:49] :) [22:43] barry: do you have time to review a test restructuring? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-430708-registry-windmill-layer/+merge/13148 [22:50] EdwinGrubbs: i can look [22:52] EdwinGrubbs: it looks like you have conflicts in your branch. see lines 8-12, 21-27 [22:52] etc [22:53] barry: hmm, l merged in trunk earlier today, but I'll do it again. [22:57] barry: oh, now I see. I was still pushing up my branch changes when launchpad computed the diff. Here is a good diff. http://pastebin.ubuntu.com/289610/ I believe the merge proposal will automatically recompute it in a little while. [22:57] EdwinGrubbs: cool [22:59] EdwinGrubbs: line 132: can't that just be range(5)? [22:59] EdwinGrubbs: also, it looks weird. why do the wait that way and not just provide a longer timeout? [22:59] EdwinGrubbs: same at line 160 [23:01] barry: I didn't realize that range() had a different behavior with just one argument. I'll do that. [23:02] EdwinGrubbs: yep, start is optional. still, why wait in a loop rather than just increase the timeout? [23:02] barry: the reason that it can't just have a longer timeout is that it successfully passes the wait and then fails on the next assertion that tests the exact same thing. I really don't know why that would happen. [23:02] yeesh [23:02] and I've been tweaking it for the last week with no better insight [23:03] EdwinGrubbs: ok. please add a comment explaining this to both spots [23:03] sure [23:04] EdwinGrubbs: cool r=me [23:05] barry: thanks === barry changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com === matsubara is now known as matsubara-afk