| mwhudson | sigh | 04:23 |
|---|---|---|
| mwhudson | anyone want to review a branch? | 04:23 |
| mwhudson | https://code.edge.launchpad.net/~mwhudson/launchpad/puller-import-hack/+merge/23370 | 04:51 |
| === henninge changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jml | hi | 10:11 |
| jml | would someone please review https://code.edge.launchpad.net/~jml/launchpad/extract-ssh-server/+merge/23350 | 10:11 |
| jml | it's a small patch that shouldn't change any behaviour | 10:12 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [-] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| maxb | I would like to submit this rather boring and small branch for review: https://code.edge.launchpad.net/~maxb/launchpad-cscvs/use-hashlib | 11:50 |
| === mrevell is now known as mrevell-lunch | ||
| danilos | allenap, hi, got a review ready for you :) https://code.edge.launchpad.net/~danilo/launchpad/bug-562450/+merge/23383 (diff is being regenerated) | 12:57 |
| === danilos changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [danilo] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara-afk is now known as matsubara | ||
| henninge | danilos: I will take a break now. I am not sure I will make the call on time, so don't wait for me. Sorry. | 13:09 |
| === wgrant changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [danilo,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === maxb changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [danilo,wgrant,maxb(cscvs)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === mrevell-lunch is now known as mrevell | ||
| intellectronica | allenap: interested in a branch for review? | 13:53 |
| intellectronica | oh wow, your queue is long | 13:54 |
| intellectronica | maybe we can find another kind soul | 13:54 |
| deryck | intellectronica, I have something for review, too. I could swap you. | 13:54 |
| intellectronica | deryck: excellent. just creating an mp. send me yours | 13:55 |
| deryck | intellectronica, thanks! https://code.edge.launchpad.net/~deryck/launchpad/bug-count-filters-503222/+merge/23386 | 13:55 |
| deryck | intellectronica, it will need UI review, too. If you want to do both. But I'm making screenshots now for the UI. | 13:55 |
| intellectronica | deryck: sure, i'll do the ui review (with sinzui mentoring) | 13:56 |
| wgrant | Two of the branches are trivial, though... | 13:57 |
| jml | I've just reviewed & approved maxb's branch | 14:00 |
| maxb | and ec2ed? :-) | 14:01 |
| jml | maxb: not yet. anyway, I think launchpad-cscvs is a pqm-tested branch | 14:01 |
| maxb | oh, sorry | 14:01 |
| maxb | pqm-submitted, then | 14:01 |
| * maxb notes that the cscvs tests don't actually _pass_ anyway | 14:02 | |
| jml | really? | 14:02 |
| intellectronica | deryck: https://code.edge.launchpad.net/~intellectronica/launchpad/bug-544794/+merge/23389 | 14:02 |
| maxb | I verified my changes didn't make them fail any worse | 14:02 |
| jml | groan | 14:03 |
| jml | it's not in 2a format | 14:03 |
| deryck | intellectronica, got it. thanks. | 14:03 |
| intellectronica | deryck: in the template, why do you need to protect against inprogress_bug_count not existing? | 14:04 |
| intellectronica | and does it behave correctly in case of 0? | 14:04 |
| deryck | intellectronica, this was the form kfogel used because somewhere this template is used where those names don't exist, IIRC. and yeah, it does work right for 0. | 14:05 |
| deryck | intellectronica, so basically I'm following all the other filters there. | 14:05 |
| intellectronica | deryck: cool. r=me for the code, then. | 14:05 |
| deryck | kfogel heard us talking about him, it seems :-) | 14:05 |
| deryck | intellectronica, thanks! | 14:05 |
| kfogel | deryck: that was coincidence -- my client was off until a second ago. | 14:06 |
| kfogel | deryck: what's up? | 14:06 |
| intellectronica | deryck: invite me for a ui review when you're ready with that | 14:07 |
| jml | ffs | 14:07 |
| deryck | kfogel, nothing, just kidding. intellectronica had asked about why I did something, and I said "copying kfogel." :-) | 14:07 |
| intellectronica | kfogel: it's easy to be a victim in the blame game when your irc client is off ;) | 14:08 |
| deryck | intellectronica, screenie is up. I can't request you as a UI reviewer. It changes the current review from code to ui if I do. | 14:09 |
| intellectronica | deryck: i'm not sure about the wording of "N In-progress bugs" | 14:09 |
| intellectronica | ot1h it fits the template of all other lines, otoh it reads a bit funny | 14:10 |
| deryck | intellectronica, ok. any suggestions? | 14:10 |
| intellectronica | "N bugs in progress" is the obvious alternative, but I don't know if it's actually better | 14:11 |
| intellectronica | deryck: let's leave it like that. changing the order will just make it harder to read. | 14:13 |
| intellectronica | deryck: ui=me* | 14:13 |
| intellectronica | sinzui: would you like to take a look? ^^^^^ | 14:13 |
| deryck | intellectronica, thanks. | 14:13 |
| sinzui | intellectronica, I can in a short while | 14:13 |
| deryck | intellectronica, concerning your MP.... does the change from security_related to setSecurityRelated have any impact on the js widget for security setting? | 14:15 |
| intellectronica | deryck: no, that's handled by the mutator in the api | 14:17 |
| intellectronica | it's only necessary to adjust the callsites internally | 14:17 |
| deryck | intellectronica, so if someone changes the security from the web ui, this method will be called and heat will change? | 14:18 |
| deryck | ah, I see now. sorry for dumbness. | 14:18 |
| intellectronica | deryck: yes, just like with bug.private, if you set bug.security_related from the api, bug.setSecurityRelated will be called | 14:18 |
| intellectronica | that's what the @mutator_for decorator in the interface does | 14:19 |
| deryck | intellectronica, I see now. Sorry for being dumb. I had never paid attention to or used mutator_for decorator myself. | 14:19 |
| deryck | that's handy | 14:19 |
| jml | maxb: done, I think. | 14:19 |
| === jml changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [danilo,wgrant] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| jml | would someone please review https://code.edge.launchpad.net/~jml/launchpad/extract-ssh-server/+merge/23350 | 14:20 |
| maxb | jml: Excellent, thanks | 14:20 |
| intellectronica | deryck: yes, would be even nicer if we could use something like that internally. i tried using property(), but it became quite awkward quickly, so i figured adjusting the existing call sites is better | 14:20 |
| jml | maxb: my pleasure | 14:21 |
| deryck | intellectronica, yup. looks good to me. r=me. | 14:21 |
| intellectronica | deryck: thanks! | 14:21 |
| === sinzui changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [danilo, wgrant, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| maxb | Could someone r or rs this as appropriate? It's purely a sourcedep revno bump. https://code.edge.launchpad.net/~maxb/launchpad/use-cscvs-r432/+merge/23395 | 15:19 |
| === noodles775 changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [danilo, wgrant, sinzui, noodles] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === noodles775 changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [danilo, wgrant, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| maxb | Thanks noodles775. Will you land it? | 15:28 |
| noodles775 | maxb: yep, it's off now :) | 15:29 |
| maxb | woot, every day, python 2.6 gets a liiiiittle bit closer :-) | 15:29 |
| noodles775 | Great! | 15:29 |
| stub | How close? It is applicable to https://bugs.edge.launchpad.net/launchpad-foundations/+bug/562828 (which is a trivial fix with Python 2.6) | 15:35 |
| mup | Bug #562828: Librarian garbage collector does not cope with symlinks <Launchpad Foundations:New> <https://launchpad.net/bugs/562828> | 15:35 |
| wgrant | Not pre-Lucid, since we don't have 2.6 on Hardy. | 15:36 |
| wgrant | But it's not far. | 15:36 |
| sinzui | deryck, did my suggestion terrify you? | 15:37 |
| deryck | sinzui, haven't looked yet, sorry. let me look.... | 15:37 |
| deryck | sinzui, I like your suggestion and am fine to do that. | 15:39 |
| === danilos is now known as daniloff | ||
| daniloff | allenap, hi, did you get any chance to restart reviewing? | 15:49 |
| allenap | daniloff: Erm, not yet; very busy :) I'll get to it as soon as I can. Edwin normally starts sometime soon too. | 15:50 |
| daniloff | allenap, cool, thanks; EdwinGrubbs, I'll be getting some food, but should be around later as well for some real OCRing :) | 15:50 |
| EdwinGrubbs | daniloff: I'll be able to start on that in a few minutes. | 15:51 |
| abentley | allenap, EdwinGrubbs could you please review https://code.edge.launchpad.net/~abentley/launchpad/recipe-security/+merge/23357 ? | 15:56 |
| === abentley changed the topic of #launchpad-reviews to: On call: allenap || reviewing: - || queue: [danilo, wgrant, sinzui, abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: On call: allenap || reviewing: adeuring || queue: [danilo, wgrant, sinzui, abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | (He asked me a while back, sorry, forgot to update the topic.) | 15:58 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: adeuring, abentley || queue: [danilo, wgrant, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| EdwinGrubbs | abentley: sure | 16:09 |
| abentley | EdwinGrubbs, thanks. | 16:09 |
| allenap | adeuring: Is there going to be another branch (or two) to add a date_expired field to IBugTask? | 16:18 |
| adeuring | allenap: argh... i forgot completely about that... | 16:19 |
| adeuring | yes, there _should_ be a branch... | 16:19 |
| allenap | adeuring: I think the bug number in the branch name is wrong. | 16:20 |
| adeuring | allenap: right... | 16:22 |
| allenap | adeuring: Line 75, could you login(product_owner) to avoid the removeSecurityProxy() call? | 16:24 |
| allenap | adeuring: Or even login_person(product_owner) | 16:25 |
| adeuring | allenap: in theory, yes. Problem that things become then more complicated: the owner can't assign another person as the supervisor, so I need to create a team, let another person join that team and finally run the test with that other user | 16:26 |
| adeuring | allenap: I was too lazy to do all that ;) | 16:26 |
| allenap | adeuring: Argh. Leave it then :) | 16:26 |
| adeuring | allenap: OK ;) | 16:26 |
| allenap | adeuring: Perhaps the removeSecurityProxy() hack could be put in LaunchpadObjectFactory.makeProduct(..., bug_supervisor=None). | 16:29 |
| adeuring | allenap: right, good idea! | 16:29 |
| === leonardr changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: adeuring, abentley || queue: [danilo, wgrant, sinzui, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| leonardr | allenap, edwin: pushing https://code.edge.launchpad.net/~leonardr/lazr.restful/561521/+merge/23404 onto the queue | 16:30 |
| === matsubara is now known as matsubara-lunch | ||
| allenap | adeuring: Line 124, the bug supervisor or the product owner should be able to call transitionToStatus(), so consider using that rather than removing the proxy. | 16:35 |
| adeuring | allenap: right | 16:36 |
| allenap | adeuring: 136s/Epired/Expired/ | 16:37 |
| adeuring | allenap: fixed | 16:37 |
| allenap | adeuring: One last thing. There's probably some doc test out there that does a similar thing to TestBugTaskEditViewStatusField. If you know where, consider removing it. | 16:39 |
| adeuring | allenap: I did not find anything like that... | 16:40 |
| adeuring | that's why I added it ;) | 16:40 |
| allenap | adeuring: Heh :) | 16:40 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: danilo, abentley || queue: [wgrant, sinzui, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: wgrant, abentley || queue: [sinzui, leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | wgrant: Want me to land lp:~wgrant/launchpad/always-use-sourcepackagename.name for you? | 16:53 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: sinzui, abentley || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| allenap | wgrant: I've sent it off to land via ec2. | 16:56 |
| === deryck is now known as deryck[lunch] | ||
| allenap | sinzui: In http://people.canonical.com/~curtis/project-without-package-suggestions.png it might work even better if no radio buttons (or the title) appeared, just the button. | 17:15 |
| sinzui | allenap, there will also be an option to say it is not packaged. | 17:16 |
| sinzui | allenap, The previous UI proposal did have button... | 17:16 |
| allenap | sinzui: Okay. Also, can_show_portlet() can return True if there are sourcepackages even when user is None. Is that right? | 17:17 |
| sinzui | Correct, that is what you see now...https://edge.launchpad.net/gedit | 17:18 |
| sinzui | The feature we are refining suggests packages when we do not know about them | 17:18 |
| sinzui | There can be a list like this: https://edge.launchpad.net/gdp | 17:19 |
| sinzui | allenap, https://edge.launchpad.net/gedit-class-browser shows just the link to choose another package, but the correct answer is Not packaged. | 17:20 |
| sinzui | allenap, The UI to support a list or packages, choose another package, or not packaged became very complicated and abentley, james_w, bac, and I agreed that we should try a single list and button | 17:21 |
| allenap | sinzui: Okay, that makes sense, thanks. | 17:22 |
| * sinzui has to rethink the model to support Not Packaged | 17:22 | |
| allenap | sinzui: Line 43 in the diff, is it safe to use 'OTHER_PACKAGE' because package.name can never be upper-case? If so, can you add a tiny comment to explain. | 17:25 |
| sinzui | hmm | 17:25 |
| sinzui | You are correct, but I think I got lucky in my name. I did not think about it | 17:26 |
| james_w | name can never have an underscore | 17:26 |
| sinzui | allenap, I was cargo culting a style I have reviewed in the past when Object() was used to create a unique and use all-caps to define the field name | 17:27 |
| sinzui | james_w, you meana source package name cannot contain underscores? | 17:28 |
| james_w | correct | 17:29 |
| james_w | for Debian/Ubuntu at least | 17:29 |
| james_w | "Package names (both source and binary, see Package, Section 5.6.7) must consist only of lower case letters (a-z), digits (0-9), plus (+) and minus (-) signs, and periods (.). They must be at least two characters long and must start with an alphanumeric character." | 17:29 |
| sinzui | ah yes, I recall someone wanted to create a gentoo package with an '/' in it | 17:30 |
| === matsubara-lunch is now known as matsubara | ||
| sinzui | That was a crucial moment where is it was clear that launchpad cannot support other distros, so should stop pretending that it does | 17:31 |
| === allenap changed the topic of #launchpad-reviews to: On call: allenap, Edwin || reviewing: -, abentley || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === allenap changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: abentley || queue: [leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| sinzui | EdwinGrubbs, rockstar: can either you you comment on the UI of my branch https://code.edge.launchpad.net/~sinzui/launchpad/project-packages-portlet-ui/+merge/23360 | 17:52 |
| rockstar | sinzui, looking | 17:53 |
| rockstar | sinzui, that's pretty straightforward. How many package suggestions can a project have? | 17:54 |
| sinzui | That's a good question. that I do not know the answer to | 17:54 |
| sinzui | I would say too much because I think I have seen 10 and I tuned out | 17:55 |
| sinzui | rockstar, 20 | 17:55 |
| sinzui | rockstar, the number and test are easy to adjust | 17:56 |
| rockstar | sinzui, I fear what it might look like with 10 different suggestions. | 17:56 |
| * sinzui think 10 is the limit | 17:56 | |
| sinzui | rockstar, no need to speculate: https://edge.launchpad.net/launchpad | 17:57 |
| sinzui | We will alway have to an option to choose another package, or say it is not packaged. I think 8 is the most we should suggest | 17:58 |
| === gary_poster is now known as gary-lunch | ||
| rockstar | sinzui, I agree. Is there code to limit the suggestions currently? | 18:02 |
| sinzui | yes, 20. The value and test are trivial fixes I can do now | 18:02 |
| * sinzui has them open | 18:02 | |
| EdwinGrubbs | abentley: r=me | 18:06 |
| rockstar | sinzui, great. With those changes, ui=me* | 18:14 |
| sinzui | rockstar, thanks. tests pass after the change | 18:15 |
| rockstar | sinzui, noodles still has to mentor me. | 18:15 |
| === deryck[lunch] is now known as deryck | ||
| * rockstar dreams of a day where he can be trusted to do UI reviews all by himself | 18:16 | |
| rockstar | Also, Launchpad just shit the bed for me. | 18:16 |
| deryck | sinzui, this kind of thing: https://devpad.canonical.com/~deryck/filters-split-up.png Or actual new portlets each in their own border outline? | 18:18 |
| sinzui | deryck, that looks great | 18:19 |
| deryck | sinzui, cool, thanks. | 18:19 |
| sinzui | I think borders would make me think too much about the filters. your pictures is good to land | 18:20 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: leonardr || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| abentley | EdwinGrubbs, thanks. | 18:29 |
| abentley | EdwinGrubbs, could you please review https://code.edge.launchpad.net/~abentley/launchpad/revert-enums/+merge/23416 ? | 18:58 |
| === abentley changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: leonardr || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === gary-lunch is now known as gary_poster | ||
| EdwinGrubbs | abentley: I can do that after lunch | 19:21 |
| abentley | EdwinGrubbs, thanks. | 19:21 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: - || queue: [abentley] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === EdwinGrubbs is now known as Edwin-lunch | ||
| leonardr | gary, maybe you'd like to take a quick look at https://code.edge.launchpad.net/~leonardr/lazr.restfulclient/no-lazr-restful/+merge/23424 ? see if i did things correctly? | 20:15 |
| gary_poster | leonardr: looking | 20:16 |
| gary_poster | leonardr: what do you think of bin/test running both? | 20:19 |
| leonardr | gary: bin/test picks up all the tests, not just the standalone ones | 20:20 |
| gary_poster | leonardr: oh I see. cool | 20:21 |
| gary_poster | leonardr: niggle: "Throughout this file, we avoid importing anything from lazr.restful..." we avoid importing anything in the module level namespace...whatever, the current wording is close enough, I guess. | 20:24 |
| leonardr | i can fix it up | 20:25 |
| gary_poster | ok cool | 20:25 |
| gary_poster | so leonardr, the only standalone test is authorizer ATM? How much of a workout is that--does that make your feel reasonably comfortable that it is at least importing all pertinent modules? | 20:26 |
| leonardr | gary: yes, definitely. it is creating a ServiceRoot object and parsing wadl | 20:27 |
| gary_poster | ok cool | 20:27 |
| gary_poster | leonardr: approved | 20:44 |
| leonardr | great | 20:44 |
| === Edwin-lunch is now known as EdwinGrubbs | ||
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: abentley || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| === matsubara is now known as matsubara-afk | ||
| james_w | EdwinGrubbs: still around? | 23:26 |
| * james_w would love a review of https://code.edge.launchpad.net/~james-w/launchpad/pipe-lint/+merge/23437 | 23:26 | |
| james_w | abentley: ^ might interest you | 23:26 |
| === james_w changed the topic of #launchpad-reviews to: On call: Edwin || reviewing: abentley || queue: [james_w(lp:~james-w/launchpad/pipe-lint)] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
| EdwinGrubbs | james_w: I'll look at that. | 23:27 |
| james_w | thanks | 23:27 |
| james_w | tiny branch | 23:27 |
| EdwinGrubbs | james_w: I thought pipelines were replacing looms. | 23:28 |
| james_w | nope | 23:28 |
| james_w | not at this point anyway | 23:28 |
| EdwinGrubbs | james_w: s/pipline/pipeline/ | 23:45 |
| EdwinGrubbs | james_w: r=me | 23:45 |
| james_w | thanks | 23:45 |
| === EdwinGrubbs changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: []|| This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
