| 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:04 |
|---|---|---|
| mwhudson | nhandler: ok | 00:05 |
| mwhudson | nhandler: let me know if you need help | 00:05 |
| nhandler | mwhudson: Will do. | 00:05 |
| === abentley1 is now known as abentley | ||
| 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:43 |
| 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:44 |
| mwhudson | nhandler: perhaps more particularly, in the interface the description should describe what the field means in the object | 02:45 |
| mwhudson | nhandler: as the description in the interface ends up here: https://edge.launchpad.net/+apidoc/#code_review_vote_reference | 02:46 |
| 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 | 02:49 |
| 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:56 |
| 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 | 03:57 |
| nhandler | mwhudson: Exactly how do I run the test? https://dev.launchpad.net/Testing only shows how to do it in ec2. | 04:01 |
| mwhudson | nhandler: ./bin/test -vvct stories/codereview i think | 04:02 |
| mwhudson | nhandler: that's a pretty terrible page | 04:02 |
| 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:04 |
| mwhudson | nhandler: try code/stories/branches | 04:06 |
| nhandler | Well, I tried actually running the first command. It started to run, but crashed with 'ImportError: No module named Crypto.Cipher | 04:10 |
| 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:13 |
| nhandler | mwhudson: Yes. It got installed when I went through the guide for setting up Launchpad | 04:14 |
| mwhudson | nhandler: hmm | 04:14 |
| mwhudson | nhandler: can you pastebin the output of the trying to run the command? | 04:15 |
| nhandler | mwhudson: http://paste.ubuntu.com/288999/ | 04:26 |
| mwhudson | nhandler: hm, that shouldn't happen, but it also shouldn't prevent you from running the other tests | 04:27 |
| mwhudson | nhandler: do you have python-crypto installed? | 04:28 |
| 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:29 |
| mwhudson | (this will run more tests than you really need to, but better than none) | 04:30 |
| stub | I assume this topic is out of date? | 04:32 |
| === stub changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| stub | Trivial and boring review waiting at https://code.edge.launchpad.net/~stub/launchpad/librarian-gc/+merge/13048 if someone can be arsed. | 04:33 |
| mwhudson | stub: luckily for you i'm working on a tests in the appserverlayer | 04:35 |
| 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:37 |
| mwhudson | nhandler: spending time to help new developers along is _always_ worth it | 04:38 |
| mwhudson | nhandler: you should be able to delete https://code.edge.launchpad.net/~nhandler/launchpad/bugfix296469/+merge/13034 | 04:48 |
| 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:49 |
| 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:53 |
| mwhudson | nhandler: a comment saying "can you look again" would have been fine | 04:54 |
| 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:55 |
| mwhudson | nhandler: oh strange | 04:56 |
| mwhudson | nhandler: i lose track between what's planned and what's implemented i'm afraid :/ | 04:56 |
| nhandler | mwhudson: Not a problem. The same thing happens to me with some of my other projects. | 04:59 |
| mwhudson | nhandler: review sent -- closer, but not quite there yet | 05:00 |
| nhandler | mwhudson: 3rd time is the charm. I just pushed the changes you suggested | 05:20 |
| mwhudson | nhandler: awesome | 05:22 |
| 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:25 |
| mwhudson | nhandler: can you supply a commit message at https://code.edge.launchpad.net/~nhandler/launchpad/bugfix296469.2/+merge/13103/+edit-commit-message ? | 05:26 |
| nhandler | mwhudson: Sure. Should it follow the format on https://dev.launchpad.net/PQMCommitMessages ? | 05:27 |
| mwhudson | nhandler: yes, basically | 05:28 |
| 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:29 |
| nhandler | mwhudson: Done | 05:33 |
| mwhudson | nhandler: i trimmed it a bit, but thanks! | 05:37 |
| 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 ;) | 05:39 | |
| === jtv is now known as jtv-sprint | ||
| wgrant | bigjools: Can I grab a tiny review from you? | 09:09 |
| 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:10 |
| bigjools | yes, it's tiny :) | 09:12 |
| bigjools | wgrant: I'll land it for you | 09:14 |
| wgrant | bigjools: Thankyou. | 09:15 |
| * bigjools wishes I could do that without making a local branch | 09:15 | |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| wgrant | bigjools: ec2 land! | 09:16 |
| bigjools | I don't use ec2 | 09:16 |
| wgrant | Pfft. | 09:17 |
| bigjools | I think I'd still need a local branch with that anyway | 09:18 |
| bigjools | I wish pqm knew about lp: urls | 09:20 |
| 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:37 |
| bigjools | BjornT: true, but not exactly user-friendly :) | 09:38 |
| 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:39 |
| BjornT | web UIs are overrated | 09:42 |
| bigjools | let's go 100% API! | 09:43 |
| jml | more ctk apps. | 09:53 |
| jml | gtk apps, I mean | 09:53 |
| bigjools | you mean qt | 10:12 |
| BjornT | let's not fight over gtk vs qt, and settle for cli instead | 10:16 |
| 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:34 |
| adeuring | BjornT: r=me | 10:37 |
| BjornT | thanks | 10:37 |
| 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 | 10:53 |
| 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:05 |
| adeuring | al-maisan: thanks for the explanation! | 11:06 |
| al-maisan | sorry for not putting that in the cover letter | 11:06 |
| al-maisan | adeuring: I will need to break for lunch in approx. 15 minutes but will be back later. | 11:13 |
| adeuring | al-maisan: enjoy lunch! | 11:14 |
| al-maisan | :) | 11:14 |
| === ursula is now known as Ursinha | ||
| mrevell | adeuring: Nice simple review for you :) https://code.edge.launchpad.net/~matthew.revell/launchpad/help-continue-button-bug-406394/+merge/13115 | 11:43 |
| adeuring | mrevell: sure. But let me first finish a review for Muharem | 11:44 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: al-maisan || queue: [mrevell] || This channel is logged: http://irclogs.ubuntu.com | ||
| mrevell | of course :) | 11:44 |
| === 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 | ||
| adeuring | mrevell-lunch: r=me | 12:55 |
| === matsubara-afk is now known as matsubara | ||
| === mrevell-lunch is now known as mrevell | ||
| mrevell | thanks adeuring | 13:47 |
| adeuring | mrevell: welcome :) | 13:47 |
| === barry changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| barry | adeuring: morning! i am both chr and ocr today, so we'll see how it goes | 14:23 |
| adeuring | barry: morning! | 14:23 |
| 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:25 |
| === barry changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: -,adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| adeuring | barry: thanks! | 14:26 |
| 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 | 15:56 |
| === adeuring changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: allenap,adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| 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:02 |
| allenap | adeuring: Comment added with the diff. Thanks. | 16:06 |
| 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:09 |
| === ursula is now known as Ursinha | ||
| 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:17 |
| adeuring | allenap: ah, right. so, r=me | 16:18 |
| allenap | adeuring: Thank you :) | 16:25 |
| adeuring | allenap: welcome :) | 16:25 |
| barry | adeuring: review sent | 16:29 |
| === barry changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: allenap,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| adeuring | barry: thanks! | 16:29 |
| === 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] | ||
| 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 :/ | 17:54 |
| === 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 | ||
| bac | barry: time for a very simple review? | 21:09 |
| 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 |
| === barry changed the topic of #launchpad-reviews to: on call: barry || reviewing: - || queue: [bac,rockstar] || This channel is logged: http://irclogs.ubuntu.com | ||
| 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:10 |
| 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:11 |
| 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 |
| === barry changed the topic of #launchpad-reviews to: on call: barry || reviewing: - || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com | ||
| rockstar | bac, yeah, that's probably a good idea. | 21:12 |
| barry | rockstar, bac agreed, though today it's been fairly light | 21:14 |
| 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:16 |
| rockstar | barry, really I should have landed this branch a while ago. | 21:17 |
| barry | rockstar: why no diff on the mp? | 21:21 |
| rockstar | barry, I think a script has exploded. | 21:21 |
| === barry changed the topic of #launchpad-reviews to: on call: barry || reviewing: rockstar || queue: [] || This channel is logged: http://irclogs.ubuntu.com | ||
| barry | rockstar: do you think this will be fixed soon? i'll be around for maybe another hour | 21:31 |
| rockstar | barry, how bout I just give you diff? | 21:32 |
| barry | rockstar: that'll work | 21:32 |
| rockstar | barry, http://pastebin.ubuntu.com/289549/ | 21:45 |
| barry | rockstar: delete line 149 or 150 | 21:48 |
| 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 | :) | 21:49 |
| 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:43 |
| barry | EdwinGrubbs: i can look | 22:50 |
| barry | EdwinGrubbs: it looks like you have conflicts in your branch. see lines 8-12, 21-27 | 22:52 |
| barry | etc | 22:52 |
| EdwinGrubbs | barry: hmm, l merged in trunk earlier today, but I'll do it again. | 22:53 |
| 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:57 |
| 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 | 22:59 |
| EdwinGrubbs | barry: I didn't realize that range() had a different behavior with just one argument. I'll do that. | 23:01 |
| 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:02 |
| barry | EdwinGrubbs: ok. please add a comment explaining this to both spots | 23:03 |
| EdwinGrubbs | sure | 23:03 |
| barry | EdwinGrubbs: cool r=me | 23:04 |
| EdwinGrubbs | barry: thanks | 23:05 |
| === 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 | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!