nhandlermwhudson: 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 changes00:04
mwhudsonnhandler: ok00:05
mwhudsonnhandler: let me know if you need help00:05
nhandlermwhudson: Will do.00:05
=== abentley1 is now known as abentley
nhandlermwhudson: 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 page02:43
mwhudsonnhandler: i'm not completely sure02:43
nhandlermwhudson: Overriding it in both locations would probably be safest.02:44
mwhudsonnhandler: 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 human02:44
mwhudsonnhandler: perhaps more particularly, in the interface the description should describe what the field means in the object02:45
mwhudsonnhandler: as the description in the interface ends up here: https://edge.launchpad.net/+apidoc/#code_review_vote_reference02:46
nhandlermwhudson: Alright, I'll update the description in the interface and add overrides in the 2 locations that the review_type is used02:49
mwhudsonnhandler: awesome, thanks02:49
nhandlermwhudson: 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
mwhudsonnhandler: you could probably run the codereview page tests locally; they shouldn't take that long03:57
mwhudsonnhandler: i'll run all the tests before landing anyway03:57
nhandlermwhudson: Exactly how do I run the test? https://dev.launchpad.net/Testing only shows how to do it in ec2.04:01
mwhudsonnhandler: ./bin/test -vvct stories/codereview i think04:02
mwhudsonnhandler: that's a pretty terrible page04:02
nhandlerlib/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
mwhudsonnhandler: try code/stories/branches04:06
nhandlerWell, I tried actually running the first command. It started to run, but crashed with 'ImportError: No module named Crypto.Cipher04:10
mwhudsonnhandler: i hope https://dev.launchpad.net/Testing is a little more sensible now04:13
mwhudsonnhandler: do you have launchpad-developer-dependencies installed?04:13
nhandlermwhudson: Yes. It got installed when I went through the guide for setting up Launchpad04:14
mwhudsonnhandler: hmm04:14
mwhudsonnhandler: can you pastebin the output of the trying to run the command?04:15
nhandlermwhudson: http://paste.ubuntu.com/288999/04:26
mwhudsonnhandler: hm, that shouldn't happen, but it also shouldn't prevent you from running the other tests04:27
mwhudsonnhandler: do you have python-crypto installed?04:28
nhandlermwhudson: That fixed the issue I had. But I get: Total: 0 tests, 0 failures, 0 errors in 0.000 seconds.04:29
nhandlerWhich does not seem right04:29
mwhudsonnhandler: try ./bin/test -vvct stories/branches04:29
mwhudson(this will run more tests than you really need to, but better than none)04:30
stubI 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
stubTrivial and boring review waiting at https://code.edge.launchpad.net/~stub/launchpad/librarian-gc/+merge/13048 if someone can be arsed.04:33
mwhudsonstub: luckily for you i'm working on a tests in the appserverlayer04:35
nhandlermwhudson: 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
nhandlerAnd I really appreciate all of the help you have provided. I have learned a lot04:37
mwhudsonnhandler: spending time to help new developers along is _always_ worth it04:38
mwhudsonnhandler: you should be able to delete https://code.edge.launchpad.net/~nhandler/launchpad/bugfix296469/+merge/1303404:48
nhandlermwhudson: 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 online04:49
mwhudsonnhandler: you can probably set the status to 'rejected' i guess04:53
mwhudsoni thought there was a way to withdraw it...04:53
mwhudsonnhandler: in general, you didn't need to make a new merge request though04:53
mwhudsonnhandler: a comment saying "can you look again" would have been fine04:54
nhandlermwhudson: 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
mwhudsonnhandler: oh strange04:56
mwhudsonnhandler: i lose track between what's planned and what's implemented i'm afraid :/04:56
nhandlermwhudson: Not a problem. The same thing happens to me with some of my other projects.04:59
mwhudsonnhandler: review sent -- closer, but not quite there yet05:00
nhandlermwhudson: 3rd time is the charm. I just pushed the changes you suggested05:20
mwhudsonnhandler: awesome05:22
mwhudsonnhandler: i see you've signed the agreement so i'll see about landing this branch05:25
* nhandler hugs mwhudson 05:25
nhandlerYou rock mwhudson !05:25
mwhudsonnhandler: can you supply a commit message at https://code.edge.launchpad.net/~nhandler/launchpad/bugfix296469.2/+merge/13103/+edit-commit-message ?05:26
nhandlermwhudson: Sure. Should it follow the format on https://dev.launchpad.net/PQMCommitMessages ?05:27
mwhudsonnhandler: yes, basically05:28
mwhudsonnhandler: you don't need to worry about the stuff in square brackets (the [r=foo] stuff)05:29
mwhudsonnhandler: the script i'll be using adds that05:29
nhandlermwhudson: Done05:33
mwhudsonnhandler: i trimmed it a bit, but thanks!05:37
nhandlermwhudson: So there isn't a need to include the bug number?05:39
mwhudsonnhandler: no, that page is a little out of date really05:39
* nhandler loves outdated documentation ;)05:39
=== jtv is now known as jtv-sprint
wgrantbigjools: Can I grab a tiny review from you?09:09
bigjoolswgrant: if it's tiny, yes09:10
wgrantbigjools: I hope it's sufficiently tiny: https://code.edge.launchpad.net/~wgrant/launchpad/bug-433739-debug-archive-urls/+merge/1310709:10
bigjoolsyes, it's tiny :)09:12
bigjoolswgrant: I'll land it for you09:14
wgrantbigjools: Thankyou.09:15
* bigjools wishes I could do that without making a local branch09:15
=== adeuring changed the topic of #launchpad-reviews to: on call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com
wgrantbigjools: ec2 land!09:16
bigjoolsI don't use ec209:16
bigjoolsI think I'd still need a local branch with that anyway09:18
bigjoolsI wish pqm knew about lp: urls09:20
BjornTbigjools: you don't need a local branch, all you need to do is to send a mail to pqm telling it to merge the branch09:37
bigjoolsBjornT: true, but not exactly user-friendly :)09:38
BjornTbigjools: another option is to hack pqm-submit not to require a local branch :)09:39
bigjoolsor put a "land me!" button on the MP09:39
BjornTweb UIs are overrated09:42
bigjoolslet's go 100% API!09:43
jmlmore ctk apps.09:53
jmlgtk apps, I mean09:53
bigjoolsyou mean qt10:12
BjornTlet's not fight over gtk vs qt, and settle for cli instead10:16
BjornTadeuring: can you review this small patch? https://code.edge.launchpad.net/~bjornt/launchpad/jscheck-all-layers/+merge/1311310:34
adeuringBjornT: sure10:34
adeuringBjornT: r=me10:37
al-maisanhello adeuring, how are things?10:53
adeuringal-maisan: fine, thanks. Need a review ;)10:53
al-maisansure :)10:53
al-maisanadeuring: 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
adeuringal-maisan: thanks for the explanation!11:06
al-maisansorry for not putting that in the cover letter11:06
al-maisanadeuring: I will need to break for lunch in approx. 15 minutes but will be back later.11:13
adeuringal-maisan: enjoy lunch!11:14
=== ursula is now known as Ursinha
mrevelladeuring: Nice simple review for you :) https://code.edge.launchpad.net/~matthew.revell/launchpad/help-continue-button-bug-406394/+merge/1311511:43
adeuringmrevell: sure. But let me first finish a review for Muharem11: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
mrevellof 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
adeuringmrevell-lunch: r=me12:55
=== matsubara-afk is now known as matsubara
=== mrevell-lunch is now known as mrevell
mrevellthanks adeuring13:47
adeuringmrevell: 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
barryadeuring: morning!  i am both chr and ocr today, so we'll see how it goes14:23
adeuringbarry: morning!14:23
adeuringbarry: 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
barryadeuring: i'll put it on the list.  i have a meeting coming up but i'll take a shot at it14:25
=== barry changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: -,adeuring || queue: [] || This channel is logged: http://irclogs.ubuntu.com
adeuringbarry: thanks!14:26
allenapadeuring: 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/1312615:56
adeuringallenap: sure15: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
allenapadeuring: I've noticed an issue with it, sorry. It's small, so I'll add paste you an incremental diff.16:02
adeuringallenap: ok, np16:02
allenapadeuring: Comment added with the diff. Thanks.16:06
adeuringallenap: 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
allenapadeuring: 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
adeuringallenap: ah, right. so, r=me16:18
allenapadeuring: Thank you :)16:25
adeuringallenap: welcome :)16:25
barryadeuring: review sent16:29
=== barry changed the topic of #launchpad-reviews to: on call: adeuring, barry || reviewing: allenap,- || queue: [] || This channel is logged: http://irclogs.ubuntu.com
adeuringbarry: 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]
barryrockstar: 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
bacbarry: time for a very simple review?21:09
rockstarbarry, I can has review?21:10
barrybac, rockstar put it on the queue.  i can get to it around chr duties i think21:10
=== barry changed the topic of #launchpad-reviews to: on call: barry || reviewing: - || queue: [bac,rockstar] || This channel is logged: http://irclogs.ubuntu.com
bacbarry: it's really more like an rs21:10
rockstarbarry, chr AND Orc duties today?  Eep.21:10
barryrockstar: yep!21:10
barrybac: looking21:10
bacin fact, i feel almost silly asking21:11
rockstarbarry, I'm wondering if that's such a good idea.21:11
barrybac: r=me :)21:11
barryrockstar: probably not :)21:11
bacbarry: thanks.21:11
rockstarbarry, I'll add it to the reviewer agenda.21:11
barryrockstar: interestingly, this is not the first time it's happened to me21:12
bacrockstar: i re-arrange my schedule so it doesn't happen21:12
=== barry changed the topic of #launchpad-reviews to: on call: barry || reviewing: - || queue: [rockstar] || This channel is logged: http://irclogs.ubuntu.com
rockstarbac, yeah, that's probably a good idea.21:12
barryrockstar, bac agreed, though today it's been fairly light21:14
rockstarbarry, https://code.edge.launchpad.net/~rockstar/launchpad/cleanup-javascript/+merge/1314321:16
rockstarbarry, 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/test21:16
rockstarbarry, really I should have landed this branch a while ago.21:17
barryrockstar: why no diff on the mp?21:21
rockstarbarry, 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
barryrockstar: do you think this will be fixed soon?  i'll be around for maybe another hour21:31
rockstarbarry, how bout I just give you diff?21:32
barryrockstar: that'll work21:32
rockstarbarry, http://pastebin.ubuntu.com/289549/21:45
barryrockstar: delete line 149 or 15021:48
rockstarbarry, ah yeah.21:49
barryrockstar: delete line 235, 236 or 23721:49
barryrockstar: nice simple branch.  everything else looks fine.  r=me21:49
rockstarbarry, thanks.21:49
* rockstar was a little whitespace crazy21:49
EdwinGrubbsbarry: do you have time to review a test restructuring? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-430708-registry-windmill-layer/+merge/1314822:43
barryEdwinGrubbs: i can look22:50
barryEdwinGrubbs: it looks like you have conflicts in your branch.  see lines 8-12, 21-2722:52
EdwinGrubbsbarry: hmm, l merged in trunk earlier today, but I'll do it again.22:53
EdwinGrubbsbarry: 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
barryEdwinGrubbs: cool22:57
barryEdwinGrubbs: line 132: can't that just be range(5)?22:59
barryEdwinGrubbs: also, it looks weird.  why do the wait that way and not just provide a longer timeout?22:59
barryEdwinGrubbs: same at line 16022:59
EdwinGrubbsbarry: I didn't realize that range() had a different behavior with just one argument. I'll do that.23:01
barryEdwinGrubbs: yep, start is optional. still, why wait in a loop rather than just increase the timeout?23:02
EdwinGrubbsbarry: 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
EdwinGrubbsand I've been tweaking it for the last week with no better insight23:02
barryEdwinGrubbs: ok.  please add a comment explaining this to both spots23:03
barryEdwinGrubbs: cool  r=me23:04
EdwinGrubbsbarry: thanks23: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!