| === intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: - || queue: [] (pqm is release-critical) || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| noodles775 | Hi intellectronica, here's one when you've time: https://code.launchpad.net/~michael.nelson/launchpad/437037-queue-timeout/+merge/12513 | 09:47 |
|---|---|---|
| === intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: noodles || queue: [] (pqm is release-critical) || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| noodles775 | intellectronica: I forgot to say on the MP, but the reason doc/build.txt is the relevant test is because it's testing the value of the estimated_buid_duration attribute of the resulting builds. | 09:50 |
| intellectronica | noodles775: nice optimization. r=me | 09:51 |
| noodles775 | intellectronica: thanks | 09:52 |
| === intellectronica changed the topic of #launchpad-reviews to: on call: intellectronica || reviewing: - || queue: [] (pqm is release-critical) || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| jtv | intellectronica: hi, I've got some stuff on the queue... you might enjoy the one about sprites. | 10:03 |
| === henninge changed the topic of #launchpad-reviews to: on call: intellectronica, henninge || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| henninge | PQM has been opened after Friday's re-roll. | 10:03 |
| jtv | Yup. I landed something over the weekend. | 10:03 |
| henninge | jtv: you workaholic! | 10:04 |
| jtv | I just wanted one more, one more! | 10:04 |
| jtv | henninge: btw maybe you could review my fix for bug 358404? | 10:04 |
| mup | Bug #358404: Set TranslationImportQueueEntry.date_status_changed in setStatus <Launchpad Translations:In Progress by jtv> <https://launchpad.net/bugs/358404> | 10:04 |
| intellectronica | jtv: cool, starting on it | 10:04 |
| jtv | intellectronica: cheers | 10:05 |
| intellectronica | jtv: r=me for the sprites branch | 10:08 |
| jtv | intellectronica: ta | 10:09 |
| jtv | intellectronica: the other one is also a small UI fixup, not quite urgent enough to push last week | 10:09 |
| jtv | maybe you'd be able to take a look at that one as well? | 10:09 |
| intellectronica | yes, looking at it now | 10:09 |
| jtv | this is just wonderful. | 10:09 |
| henninge | jtv: does setStatus only get called if the status actually changed? Or should the mehod check for that and not update if unchanged?= | 10:10 |
| jtv | henninge: I don't think there's anything to gain by making it that smart... The UI already checks for non-changes. | 10:11 |
| jtv | henninge: otoh the date gets updated explicitly when a re-upload refreshes a Needs Review entry without changing its state, so there's no particular reason not to check for non-changes either. | 10:12 |
| henninge | jtv: but you mentioned API .. | 10:12 |
| intellectronica | jtv: r=me for that too | 10:12 |
| jtv | intellectronica: getting things done! Thanks. Does that include UI? | 10:12 |
| henninge | jtv: ok, if you say so ... ;-) | 10:12 |
| intellectronica | jtv: yes | 10:13 |
| jtv | henninge: you've got a good point in that we might be doing non-updates that we don't realize. Not sure if we want to notice those or just be able to ignore them. :) | 10:13 |
| jtv | intellectronica: spasiba | 10:13 |
| jtv | intellectronica: ah, you reviewed the one that henninge was looking at! | 10:19 |
| * jtv should have been clearer | 10:19 | |
| * henninge thought it was clear .. | 10:20 | |
| henninge | jtv, intellectronica: never mind, I still have a nasty bug in bzr imports to look at ... | 10:20 |
| intellectronica | jtv: great, your work will benefit from the scrutiny of not one but two reviewers! | 10:20 |
| * jtv trembles in joyful anticipation | 10:21 | |
| jtv | "If your code is good, you have nothing to fear. Do you feel any fear, citizen?" | 10:21 |
| intellectronica | jtv: so shall i look at bug-433989 now? | 10:21 |
| jtv | intellectronica: that's what I was hoping for, yes, since it's UI. | 10:21 |
| intellectronica | jtv: cool, grabbing it now | 10:22 |
| intellectronica | jtv: r=me ui=me | 10:36 |
| jtv | intellectronica: splended, thanks! | 10:37 |
| jtv | *splendid | 10:37 |
| gmb | intellectronica: Could you do a code and ui review of https://code.edge.launchpad.net/~gmb/launchpad/ajaxify-branch-linking/+merge/12514 please? It's the first part of the branch linking ajax work. | 11:21 |
| intellectronica | gmb: sure | 11:22 |
| gmb | Ta | 11:22 |
| gmb | intellectronica: Argh, it's proposed for db-devel, not devel. Bugger... I'll generate one for you now. | 11:37 |
| intellectronica | oh right | 11:37 |
| gmb | intellectronica: Diff pasted. | 11:37 |
| gmb | (into the mp) | 11:38 |
| intellectronica | danke | 11:38 |
| === henninge_ is now known as henninge | ||
| adeuring | intellectronica, henninge: could one of you review this small branch: https:/ ?/code.edge.launchpad.net/~adeuring/launchpad/bug-438024-parse-dmi-data/+merge/12518 | 12:46 |
| adeuring | https://code.edge.launchpad.net/~adeuring/launchpad/bug-438024-parse-dmi-data/+merge/12518 | 12:46 |
| jtatum | I have a small wording correction in a bzr branch and was wondering if someone could review it. This is my first merge proposal to launchpad and I don't know the process. | 13:13 |
| intellectronica | adeuring: starting on it now | 13:13 |
| intellectronica | jtatum: hi! | 13:13 |
| jtatum | intellectronica, hi :) | 13:13 |
| intellectronica | jtatum: so what's your fix about? and thanks for joining the effort :) | 13:14 |
| intellectronica | jtatum: you can find some info on how to contribute at https://dev.launchpad.net/PatchSubmission (and on the dev wiki in general) | 13:14 |
| jtatum | intellectronica, it's an embarrassingly trivial wording change for bug 435320 | 13:14 |
| mup | Bug #435320: +branding page text needs improvement <post-3-ui-cleanup> <story-logos> <trivial> <ui> <Launchpad Registry:In Progress by jtatum> <https://launchpad.net/bugs/435320> | 13:14 |
| intellectronica | jtatum: also, the on-call reviewers, which change from time to time (henninge and myself are manning the current shift) can always help | 13:15 |
| jtatum | intellectronica, great :) i read this and saw the various templates for proposals, but couldn't divine how to run lint or any other prerequisites I'd need to take care of for the merge | 13:16 |
| intellectronica | jtatum: to run lint just type make lint in the branch | 13:17 |
| intellectronica | jtatum: once you're happy with your change, propose it for merge into the launchpad/devel branch and ask for it to be reviewed | 13:17 |
| intellectronica | that means me :) i will review your branch after you create the merge proposal and ping me | 13:18 |
| intellectronica | jtatum: if it's all good, i'll land the branch on your behalf, if there are things to fix, we'll talk about them | 13:18 |
| jtatum | intellectronica, fantastic :) proposing it now. thanks :) | 13:19 |
| === matsubara-afk is now known as matsubara | ||
| intellectronica | adeuring: s/ndes/nodes other than that everything looks great. r=me | 13:23 |
| jtatum | intellectronica, submitted and diff generated: https://code.edge.launchpad.net/~jtatum/launchpad/bug-435320/+merge/12519 | 13:27 |
| intellectronica | jtatum: nice fix, merge-approved :) | 13:28 |
| jtatum | intellectronica, neat! thanks. hopefully my next merge will be more substantial ;) | 13:29 |
| intellectronica | jtatum: since you can't merge branches into the launchpad tree i will land it on your behalf (after testing it, so it will take ~3 hours before it lands) | 13:29 |
| jtatum | intellectronica, no worries. thanks again. | 13:30 |
| intellectronica | jtatum: great. if you want ideas on interesting things to work on, or if you're starting to work on something and want to chat about how to go about it, this is the place to come to | 13:30 |
| jtatum | intellectronica, I believe I will :) incidentally I have submitted the canonical contributors agreement for LP, in case that comes up on a checklist somewhere | 13:33 |
| adeuring | intellectronica: thanks! | 13:34 |
| intellectronica | jtatum: excellent. i was just about to forget to ask you about that ;) | 13:34 |
| intellectronica | whoa, so it looks like the whole ec2test manoeuvre changed | 13:36 |
| noodles775 | intellectronica or henninge : test fix for current buildbot breakage? http://pastebin.ubuntu.com/280368/ | 14:02 |
| henninge | noodles775: looking | 14:03 |
| * intellectronica reads | 14:03 | |
| intellectronica | henninge: i'll leave it to you then and go eat | 14:03 |
| henninge | intellectronica: enjoy it! | 14:04 |
| noodles775 | henninge: I've got two questions - first, is there a standard way to match pretty quotes in pagetests - I've used ... here but have also seen '?' - happy to change them. | 14:04 |
| noodles775 | henninge: the second, I'll chat with Edwin afterwards, but I don't really agree with the way the labels are being used here (effectively using the leaf breadcrum 'page_title' as the label), but I'll send an email to Edwin about that. | 14:05 |
| henninge | noodles775: I don't know. I have also seen backslashreplace used or actual pretty quotes in the test. | 14:05 |
| henninge | *in tests* | 14:05 |
| noodles775 | henninge: I've seen the actual pretty quotes in view tests, but hadn't noticed in in pagetests, but yeah, lots of options. | 14:06 |
| henninge | noodles775: would be too much for a testfix. this looks good. r=me | 14:06 |
| noodles775 | henninge: great, thanks. | 14:06 |
| henninge | Get us up and running again! | 14:06 |
| henninge | ;-) | 14:06 |
| === abentley changed the topic of #launchpad-reviews to: on call: intellectronica, henninge, abentley || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| === jtv1 is now known as jtv | ||
| henninge | noodles775: have you not landed your testfix branch yet? | 14:37 |
| henninge | Hi abentley ! | 14:37 |
| noodles775 | henninge: I have indeed (r9581), but it seems buildbot hasn't started? | 14:37 |
| * noodles775 checks with mthaddon | 14:38 | |
| henninge | noodles775: it was just restarted it looks | 14:38 |
| noodles775 | henninge: really? where are you seeing that? I'm looking at https://lpbuildbot.canonical.com/waterfall | 14:38 |
| henninge | noodles775: me, too. master shutdown - master started ... | 14:39 |
| noodles775 | yep, that was a while ago though... | 14:39 |
| henninge | oh, me not looking at time properly, then | 14:40 |
| henninge | yeah, 3 hrs ago ... | 14:41 |
| abentley | henninge: Hi | 14:41 |
| henninge | abentley: what do you make of this? http://paste.ubuntu.com/280287/ | 14:42 |
| henninge | abentley: should I file a bug or could there be some pattern, I don't recognise? | 14:42 |
| henninge | and s/toople/tuple/ | 14:43 |
| henninge | ;) | 14:43 |
| abentley | henninge: These are the same type of tree? They're not one RevisionTree and one WorkingTree? | 14:43 |
| henninge | abentley: hm | 14:44 |
| henninge | abentley: they should be the same. | 14:44 |
| henninge | abentley: it happens when running the same script (rosetta-branches) on different branches. | 14:44 |
| abentley | henninge: It's a bug. The behaviour of get_file_text on directories should be consistent. | 14:45 |
| henninge | abentley: ok, I'll file it. | 14:46 |
| henninge | abentley: thanks | 14:46 |
| abentley | henninge: np | 14:46 |
| abentley | henninge: Please specify which tree type is causing this. | 14:47 |
| === EdwinGrubbs2 is now known as EdwinGrubbs | ||
| * henninge looks that up | 14:48 | |
| henninge | abentley: it's a RevisionTree. I'll put that in the bug. | 14:48 |
| abentley | henninge: great. | 14:49 |
| abentley | henninge: A possible cause would be that one branch is in 2a format and the other is in a different format. | 14:49 |
| abentley | henninge: Since 2a introduces a different way of representing tree contents. | 14:50 |
| EdwinGrubbs | intellectronica, henninge, abentley: Can you review my branch to get pqm out of testfix mode? | 15:23 |
| EdwinGrubbs | http://pastebin.ubuntu.com/280424/ | 15:23 |
| henninge | EdwinGrubbs: noodles775_ already did that. | 15:23 |
| henninge | EdwinGrubbs: He claims to have landed it ... ;-) | 15:23 |
| abentley | EdwinGrubbs: Sorry, debugging the puller. | 15:23 |
| EdwinGrubbs | noodles775_: thanks for fixing my errors | 15:24 |
| noodles775_ | EdwinGrubbs: np! I sent you an email with some related thoughts. | 15:25 |
| === Ursinha is now known as Ursinha-nom | ||
| === matsubara is now known as matsubara-lunch | ||
| === henninge changed the topic of #launchpad-reviews to: on call: intellectronica, abentley || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| === Ursinha-nom is now known as Ursinha | ||
| === intellectronica changed the topic of #launchpad-reviews to: on call: abentley || reviewing: -, - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| === matsubara-lunch is now known as matsubara | ||
| === EdwinGrubbs is now known as Edwin-lunch | ||
| leonardr | abentley, i request a review for https://code.edge.launchpad.net/~leonardr/lazr.restful/basic-auth/+merge/12538 | 19:08 |
| abentley | leonardr: Sorry, I'm a bit distracted by problems with the branch puller. | 19:11 |
| leonardr | abentley, np | 19:11 |
| === abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| abentley | leonardr: check_foo doesn't suggest to me that it returns a value. What would you think about renaming it authenticate_user? | 19:24 |
| leonardr | ok, sure | 19:25 |
| abentley | leonardr: is the environ parameter to self.application allowed to contain arbitrary objects? | 19:29 |
| abentley | leonardr: In the check_credentials docstring, I believe ReST requires an indent on the second line of ":return:". | 19:33 |
| abentley | leonardr: There is no documentation of the __init__ parameters or the ivars. Could you please document one of them? | 19:34 |
| leonardr | abentley: yes, wsgi lets you put anything you want in environ | 19:34 |
| abentley | leonardr: The name "self.regex" doesn't communicate what the regex is for. What about secure_paths or secure_path_pattern or include_paths or something? | 19:36 |
| leonardr | abentley, wilco | 19:37 |
| abentley | leonardr: Aside from that, looks good to me. | 19:42 |
| abentley | leonardr: random thought: would it be more WSGIish to pass check_credentials in as a callback? | 19:48 |
| leonardr | abentley: i don't think so, but it would probably be nicer code | 19:54 |
| === abentley changed the topic of #launchpad-reviews to: on call: abentley || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| EdwinGrubbs | abentley: can you review a super simple branch for me? https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-421898-cross-site-timeline-graph/+merge/12551 | 22:06 |
| abentley | EdwinGrubbs: r=me | 22:07 |
| === abentley changed the topic of #launchpad-reviews to: on call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || | ||
| EdwinGrubbs | abentley: thanks | 22:20 |
| === matsubara__ is now known as matsubara-afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!