[00:04] <nhandler> 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] <mwhudson> nhandler: ok
[00:05] <mwhudson> nhandler: let me know if you need help
[00:05] <nhandler> mwhudson: Will do.
[02:43] <nhandler> 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] <mwhudson> nhandler: i'm not completely sure
[02:44] <nhandler> mwhudson: Overriding it in both locations would probably be safest.
[02:44] <mwhudson> 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] <mwhudson> nhandler: perhaps more particularly, in the interface the description should describe what the field means in the object
[02:46] <mwhudson> nhandler: as the description in the interface ends up here: https://edge.launchpad.net/+apidoc/#code_review_vote_reference
[02:49] <nhandler> 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] <mwhudson> nhandler: awesome, thanks
[03:56] <nhandler> 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] <mwhudson> nhandler: you could probably run the codereview page tests locally; they shouldn't take that long
[03:57] <mwhudson> nhandler: i'll run all the tests before landing anyway
[04:01] <nhandler> mwhudson: Exactly how do I run the test? https://dev.launchpad.net/Testing only shows how to do it in ec2.
[04:02] <mwhudson> nhandler: ./bin/test -vvct stories/codereview i think
[04:02] <mwhudson> nhandler: that's a pretty terrible page
[04:04] <nhandler> 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] <mwhudson> hmm!
[04:06] <mwhudson> nhandler: try code/stories/branches
[04:10] <nhandler> Well, I tried actually running the first command. It started to run, but crashed with 'ImportError: No module named Crypto.Cipher
[04:13] <mwhudson> nhandler: i hope https://dev.launchpad.net/Testing is a little more sensible now
[04:13] <mwhudson> nhandler: do you have launchpad-developer-dependencies installed?
[04:14] <nhandler> mwhudson: Yes. It got installed when I went through the guide for setting up Launchpad
[04:14] <mwhudson> nhandler: hmm
[04:15] <mwhudson> nhandler: can you pastebin the output of the trying to run the command?
[04:26] <nhandler> mwhudson: http://paste.ubuntu.com/288999/
[04:27] <mwhudson> nhandler: hm, that shouldn't happen, but it also shouldn't prevent you from running the other tests
[04:28] <mwhudson> nhandler: do you have python-crypto installed?
[04:29] <nhandler> mwhudson: That fixed the issue I had. But I get: Total: 0 tests, 0 failures, 0 errors in 0.000 seconds.
[04:29] <nhandler> Which does not seem right
[04:29] <mwhudson> nhandler: try ./bin/test -vvct stories/branches
[04:30] <mwhudson> (this will run more tests than you really need to, but better than none)
[04:32] <stub> I assume this topic is out of date?
[04:33] <stub> Trivial and boring review waiting at https://code.edge.launchpad.net/~stub/launchpad/librarian-gc/+merge/13048 if someone can be arsed.
[04:35] <mwhudson> stub: luckily for you i'm working on a tests in the appserverlayer
[04:37] <nhandler> 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] <nhandler> And I really appreciate all of the help you have provided. I have learned a lot
[04:38] <mwhudson> nhandler: spending time to help new developers along is _always_ worth it
[04:48] <mwhudson> nhandler: you should be able to delete https://code.edge.launchpad.net/~nhandler/launchpad/bugfix296469/+merge/13034
[04:49] <nhandler> 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] <mwhudson> nhandler: you can probably set the status to 'rejected' i guess
[04:53] <mwhudson> i thought there was a way to withdraw it...
[04:53] <mwhudson> nhandler: in general, you didn't need to make a new merge request though
[04:54] <mwhudson> nhandler: a comment saying "can you look again" would have been fine
[04:55] <nhandler> 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] <mwhudson> nhandler: oh strange
[04:56] <mwhudson> nhandler: i lose track between what's planned and what's implemented i'm afraid :/
[04:59] <nhandler> mwhudson: Not a problem. The same thing happens to me with some of my other projects.
[05:00] <mwhudson> nhandler: review sent -- closer, but not quite there yet
[05:20] <nhandler> mwhudson: 3rd time is the charm. I just pushed the changes you suggested
[05:22] <mwhudson> nhandler: awesome
[05:25] <mwhudson> nhandler: i see you've signed the agreement so i'll see about landing this branch
[05:25]  * nhandler hugs mwhudson 
[05:25] <nhandler> You rock mwhudson !
[05:26] <mwhudson> nhandler: can you supply a commit message at https://code.edge.launchpad.net/~nhandler/launchpad/bugfix296469.2/+merge/13103/+edit-commit-message ?
[05:27] <nhandler> mwhudson: Sure. Should it follow the format on https://dev.launchpad.net/PQMCommitMessages ?
[05:28] <mwhudson> nhandler: yes, basically
[05:29] <mwhudson> nhandler: you don't need to worry about the stuff in square brackets (the [r=foo] stuff)
[05:29] <mwhudson> nhandler: the script i'll be using adds that
[05:33] <nhandler> mwhudson: Done
[05:37] <mwhudson> nhandler: i trimmed it a bit, but thanks!
[05:39] <nhandler> mwhudson: So there isn't a need to include the bug number?
[05:39] <mwhudson> nhandler: no, that page is a little out of date really
[05:39]  * nhandler loves outdated documentation ;)
[09:09] <wgrant> bigjools: Can I grab a tiny review from you?
[09:10] <bigjools> wgrant: if it's tiny, yes
[09:10] <wgrant> bigjools: I hope it's sufficiently tiny: https://code.edge.launchpad.net/~wgrant/launchpad/bug-433739-debug-archive-urls/+merge/13107
[09:12] <bigjools> yes, it's tiny :)
[09:14] <bigjools> wgrant: I'll land it for you
[09:15] <wgrant> bigjools: Thankyou.
[09:15]  * bigjools wishes I could do that without making a local branch
[09:16] <wgrant> bigjools: ec2 land!
[09:16] <bigjools> I don't use ec2
[09:17] <wgrant> Pfft.
[09:18] <bigjools> I think I'd still need a local branch with that anyway
[09:20] <bigjools> I wish pqm knew about lp: urls
[09:37] <BjornT> 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] <bigjools> BjornT: true, but not exactly user-friendly :)
[09:39] <BjornT> bigjools: another option is to hack pqm-submit not to require a local branch :)
[09:39] <bigjools> or put a "land me!" button on the MP
[09:42] <BjornT> web UIs are overrated
[09:43] <bigjools> let's go 100% API!
[09:53] <jml> more ctk apps.
[09:53] <jml> gtk apps, I mean
[10:12] <bigjools> you mean qt
[10:16] <BjornT> let's not fight over gtk vs qt, and settle for cli instead
[10:34] <BjornT> adeuring: can you review this small patch? https://code.edge.launchpad.net/~bjornt/launchpad/jscheck-all-layers/+merge/13113
[10:34] <adeuring> BjornT: sure
[10:37] <adeuring> BjornT: r=me
[10:37] <BjornT> thanks
[10:53] <al-maisan> hello adeuring, how are things?
[10:53] <adeuring> al-maisan: fine, thanks. Need a review ;)
[10:53] <al-maisan> sure :)
[10:53] <al-maisan> https://code.edge.launchpad.net/~al-maisan/launchpad/unembargo-443075/+merge/13114
[11:05] <al-maisan> 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] <adeuring> al-maisan: thanks for the explanation!
[11:06] <al-maisan> sorry for not putting that in the cover letter
[11:13] <al-maisan> adeuring: I will need to break for lunch in approx. 15 minutes but will be back later.
[11:14] <adeuring> al-maisan: enjoy lunch!
[11:14] <al-maisan> :)
[11:43] <mrevell> adeuring: Nice simple review for you :) https://code.edge.launchpad.net/~matthew.revell/launchpad/help-continue-button-bug-406394/+merge/13115
[11:44] <adeuring> mrevell: sure. But let me first finish a review for Muharem
[11:44] <mrevell> of course :)
[12:55] <adeuring> mrevell-lunch: r=me
[13:47] <mrevell> thanks adeuring
[13:47] <adeuring> mrevell: welcome :)
[14:23] <barry> adeuring: morning!  i am both chr and ocr today, so we'll see how it goes
[14:23] <adeuring> barry: morning!
[14:25] <adeuring> 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] <barry> adeuring: i'll put it on the list.  i have a meeting coming up but i'll take a shot at it
[14:26] <adeuring> barry: thanks!
[15:56] <allenap> 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] <adeuring> allenap: sure
[16:02] <allenap> adeuring: I've noticed an issue with it, sorry. It's small, so I'll add paste you an incremental diff.
[16:02] <adeuring> allenap: ok, np
[16:06] <allenap> adeuring: Comment added with the diff. Thanks.
[16:09] <adeuring> 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?
[16:17] <allenap> 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] <adeuring> allenap: ah, right. so, r=me
[16:25] <allenap> adeuring: Thank you :)
[16:25] <adeuring> allenap: welcome :)
[16:29] <barry> adeuring: review sent
[16:29] <adeuring> barry: thanks!
[17:54] <barry> 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 :/
[21:09] <bac> barry: time for a very simple review?
[21:10] <rockstar> barry, I can has review?
[21:10] <barry> bac, rockstar put it on the queue.  i can get to it around chr duties i think
[21:10] <bac> https://code.edge.launchpad.net/~bac/launchpad/bug-435599/+merge/13141
[21:10] <bac> barry: it's really more like an rs
[21:10] <rockstar> barry, chr AND Orc duties today?  Eep.
[21:10] <barry> rockstar: yep!
[21:10] <barry> bac: looking
[21:11] <bac> in fact, i feel almost silly asking
[21:11] <rockstar> barry, I'm wondering if that's such a good idea.
[21:11] <barry> bac: r=me :)
[21:11] <barry> rockstar: probably not :)
[21:11] <bac> barry: thanks.
[21:11] <rockstar> barry, I'll add it to the reviewer agenda.
[21:12] <barry> rockstar: interestingly, this is not the first time it's happened to me
[21:12] <bac> rockstar: i re-arrange my schedule so it doesn't happen
[21:12] <rockstar> bac, yeah, that's probably a good idea.
[21:14] <barry> rockstar, bac agreed, though today it's been fairly light
[21:16] <rockstar> barry, https://code.edge.launchpad.net/~rockstar/launchpad/cleanup-javascript/+merge/13143
[21:16] <rockstar> 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] <rockstar> barry, really I should have landed this branch a while ago.
[21:21] <barry> rockstar: why no diff on the mp?
[21:21] <rockstar> barry, I think a script has exploded.
[21:31] <barry> rockstar: do you think this will be fixed soon?  i'll be around for maybe another hour
[21:32] <rockstar> barry, how bout I just give you diff?
[21:32] <barry> rockstar: that'll work
[21:45] <rockstar> barry, http://pastebin.ubuntu.com/289549/
[21:48] <barry> rockstar: delete line 149 or 150
[21:49] <rockstar> barry, ah yeah.
[21:49] <barry> rockstar: delete line 235, 236 or 237
[21:49] <barry> rockstar: nice simple branch.  everything else looks fine.  r=me
[21:49] <rockstar> barry, thanks.
[21:49]  * rockstar was a little whitespace crazy
[21:49] <barry> :)
[22:43] <EdwinGrubbs> 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] <barry> EdwinGrubbs: i can look
[22:52] <barry> EdwinGrubbs: it looks like you have conflicts in your branch.  see lines 8-12, 21-27
[22:52] <barry> etc
[22:53] <EdwinGrubbs> barry: hmm, l merged in trunk earlier today, but I'll do it again.
[22:57] <EdwinGrubbs> 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] <barry> EdwinGrubbs: cool
[22:59] <barry> EdwinGrubbs: line 132: can't that just be range(5)?
[22:59] <barry> EdwinGrubbs: also, it looks weird.  why do the wait that way and not just provide a longer timeout?
[22:59] <barry> EdwinGrubbs: same at line 160
[23:01] <EdwinGrubbs> barry: I didn't realize that range() had a different behavior with just one argument. I'll do that.
[23:02] <barry> EdwinGrubbs: yep, start is optional. still, why wait in a loop rather than just increase the timeout?
[23:02] <EdwinGrubbs> 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] <barry> yeesh
[23:02] <EdwinGrubbs> and I've been tweaking it for the last week with no better insight
[23:03] <barry> EdwinGrubbs: ok.  please add a comment explaining this to both spots
[23:03] <EdwinGrubbs> sure
[23:04] <barry> EdwinGrubbs: cool  r=me
[23:05] <EdwinGrubbs> barry: thanks