[04:23] sigh [04:23] anyone want to review a branch? [04:51] https://code.edge.launchpad.net/~mwhudson/launchpad/puller-import-hack/+merge/23370 === 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 [10:11] hi [10:11] would someone please review https://code.edge.launchpad.net/~jml/launchpad/extract-ssh-server/+merge/23350 [10:12] it's a small patch that shouldn't change any behaviour === 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 [11:50] I would like to submit this rather boring and small branch for review: https://code.edge.launchpad.net/~maxb/launchpad-cscvs/use-hashlib === mrevell is now known as mrevell-lunch [12:57] allenap, hi, got a review ready for you :) https://code.edge.launchpad.net/~danilo/launchpad/bug-562450/+merge/23383 (diff is being regenerated) === 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 [13:09] 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. === 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 [13:53] allenap: interested in a branch for review? [13:54] oh wow, your queue is long [13:54] maybe we can find another kind soul [13:54] intellectronica, I have something for review, too. I could swap you. [13:55] deryck: excellent. just creating an mp. send me yours [13:55] intellectronica, thanks! https://code.edge.launchpad.net/~deryck/launchpad/bug-count-filters-503222/+merge/23386 [13:55] intellectronica, it will need UI review, too. If you want to do both. But I'm making screenshots now for the UI. [13:56] deryck: sure, i'll do the ui review (with sinzui mentoring) [13:57] Two of the branches are trivial, though... [14:00] I've just reviewed & approved maxb's branch [14:01] and ec2ed? :-) [14:01] maxb: not yet. anyway, I think launchpad-cscvs is a pqm-tested branch [14:01] oh, sorry [14:01] pqm-submitted, then [14:02] * maxb notes that the cscvs tests don't actually _pass_ anyway [14:02] really? [14:02] deryck: https://code.edge.launchpad.net/~intellectronica/launchpad/bug-544794/+merge/23389 [14:02] I verified my changes didn't make them fail any worse [14:03] groan [14:03] it's not in 2a format [14:03] intellectronica, got it. thanks. [14:04] deryck: in the template, why do you need to protect against inprogress_bug_count not existing? [14:04] and does it behave correctly in case of 0? [14:05] 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] intellectronica, so basically I'm following all the other filters there. [14:05] deryck: cool. r=me for the code, then. [14:05] kfogel heard us talking about him, it seems :-) [14:05] intellectronica, thanks! [14:06] deryck: that was coincidence -- my client was off until a second ago. [14:06] deryck: what's up? [14:07] deryck: invite me for a ui review when you're ready with that [14:07] ffs [14:07] kfogel, nothing, just kidding. intellectronica had asked about why I did something, and I said "copying kfogel." :-) [14:08] kfogel: it's easy to be a victim in the blame game when your irc client is off ;) [14:09] 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] deryck: i'm not sure about the wording of "N In-progress bugs" [14:10] ot1h it fits the template of all other lines, otoh it reads a bit funny [14:10] intellectronica, ok. any suggestions? [14:11] "N bugs in progress" is the obvious alternative, but I don't know if it's actually better [14:13] deryck: let's leave it like that. changing the order will just make it harder to read. [14:13] deryck: ui=me* [14:13] sinzui: would you like to take a look? ^^^^^ [14:13] intellectronica, thanks. [14:13] intellectronica, I can in a short while [14:15] intellectronica, concerning your MP.... does the change from security_related to setSecurityRelated have any impact on the js widget for security setting? [14:17] deryck: no, that's handled by the mutator in the api [14:17] it's only necessary to adjust the callsites internally [14:18] intellectronica, so if someone changes the security from the web ui, this method will be called and heat will change? [14:18] ah, I see now. sorry for dumbness. [14:18] deryck: yes, just like with bug.private, if you set bug.security_related from the api, bug.setSecurityRelated will be called [14:19] that's what the @mutator_for decorator in the interface does [14:19] intellectronica, I see now. Sorry for being dumb. I had never paid attention to or used mutator_for decorator myself. [14:19] that's handy [14:19] maxb: done, I think. === 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 [14:20] would someone please review https://code.edge.launchpad.net/~jml/launchpad/extract-ssh-server/+merge/23350 [14:20] jml: Excellent, thanks [14:20] 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:21] maxb: my pleasure [14:21] intellectronica, yup. looks good to me. r=me. [14:21] deryck: thanks! === 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 [15:19] 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 === 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 [15:28] Thanks noodles775. Will you land it? [15:29] maxb: yep, it's off now :) [15:29] woot, every day, python 2.6 gets a liiiiittle bit closer :-) [15:29] Great! [15:35] 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] Bug #562828: Librarian garbage collector does not cope with symlinks [15:36] Not pre-Lucid, since we don't have 2.6 on Hardy. [15:36] But it's not far. [15:37] deryck, did my suggestion terrify you? [15:37] sinzui, haven't looked yet, sorry. let me look.... [15:39] sinzui, I like your suggestion and am fine to do that. === danilos is now known as daniloff [15:49] allenap, hi, did you get any chance to restart reviewing? [15:50] daniloff: Erm, not yet; very busy :) I'll get to it as soon as I can. Edwin normally starts sometime soon too. [15:50] allenap, cool, thanks; EdwinGrubbs, I'll be getting some food, but should be around later as well for some real OCRing :) [15:51] daniloff: I'll be able to start on that in a few minutes. [15:56] allenap, EdwinGrubbs could you please review https://code.edge.launchpad.net/~abentley/launchpad/recipe-security/+merge/23357 ? === 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 [15:58] (He asked me a while back, sorry, forgot to update the topic.) === 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 [16:09] abentley: sure [16:09] EdwinGrubbs, thanks. [16:18] adeuring: Is there going to be another branch (or two) to add a date_expired field to IBugTask? [16:19] allenap: argh... i forgot completely about that... [16:19] yes, there _should_ be a branch... [16:20] adeuring: I think the bug number in the branch name is wrong. [16:22] allenap: right... [16:24] adeuring: Line 75, could you login(product_owner) to avoid the removeSecurityProxy() call? [16:25] adeuring: Or even login_person(product_owner) [16:26] 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] allenap: I was too lazy to do all that ;) [16:26] adeuring: Argh. Leave it then :) [16:26] allenap: OK ;) [16:29] adeuring: Perhaps the removeSecurityProxy() hack could be put in LaunchpadObjectFactory.makeProduct(..., bug_supervisor=None). [16:29] allenap: right, good idea! === 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 [16:30] allenap, edwin: pushing https://code.edge.launchpad.net/~leonardr/lazr.restful/561521/+merge/23404 onto the queue === matsubara is now known as matsubara-lunch [16:35] 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:36] allenap: right [16:37] adeuring: 136s/Epired/Expired/ [16:37] allenap: fixed [16:39] 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:40] allenap: I did not find anything like that... [16:40] that's why I added it ;) [16:40] adeuring: Heh :) === 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 [16:53] wgrant: Want me to land lp:~wgrant/launchpad/always-use-sourcepackagename.name for you? === 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 [16:56] wgrant: I've sent it off to land via ec2. === deryck is now known as deryck[lunch] [17:15] 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:16] allenap, there will also be an option to say it is not packaged. [17:16] allenap, The previous UI proposal did have button... [17:17] sinzui: Okay. Also, can_show_portlet() can return True if there are sourcepackages even when user is None. Is that right? [17:18] Correct, that is what you see now...https://edge.launchpad.net/gedit [17:18] The feature we are refining suggests packages when we do not know about them [17:19] There can be a list like this: https://edge.launchpad.net/gdp [17:20] allenap, https://edge.launchpad.net/gedit-class-browser shows just the link to choose another package, but the correct answer is Not packaged. [17:21] 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:22] sinzui: Okay, that makes sense, thanks. [17:22] * sinzui has to rethink the model to support Not Packaged [17:25] 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] hmm [17:26] You are correct, but I think I got lucky in my name. I did not think about it [17:26] name can never have an underscore [17:27] 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:28] james_w, you meana source package name cannot contain underscores? [17:29] correct [17:29] for Debian/Ubuntu at least [17:29] "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:30] ah yes, I recall someone wanted to create a gentoo package with an '/' in it === matsubara-lunch is now known as matsubara [17:31] That was a crucial moment where is it was clear that launchpad cannot support other distros, so should stop pretending that it does === 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 [17:52] 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:53] sinzui, looking [17:54] sinzui, that's pretty straightforward. How many package suggestions can a project have? [17:54] That's a good question. that I do not know the answer to [17:55] I would say too much because I think I have seen 10 and I tuned out [17:55] rockstar, 20 [17:56] rockstar, the number and test are easy to adjust [17:56] sinzui, I fear what it might look like with 10 different suggestions. [17:56] * sinzui think 10 is the limit [17:57] rockstar, no need to speculate: https://edge.launchpad.net/launchpad [17:58] 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 === gary_poster is now known as gary-lunch [18:02] sinzui, I agree. Is there code to limit the suggestions currently? [18:02] yes, 20. The value and test are trivial fixes I can do now [18:02] * sinzui has them open [18:06] abentley: r=me [18:14] sinzui, great. With those changes, ui=me* [18:15] rockstar, thanks. tests pass after the change [18:15] sinzui, noodles still has to mentor me. === deryck[lunch] is now known as deryck [18:16] * rockstar dreams of a day where he can be trusted to do UI reviews all by himself [18:16] Also, Launchpad just shit the bed for me. [18:18] 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:19] deryck, that looks great [18:19] sinzui, cool, thanks. [18:20] I think borders would make me think too much about the filters. your pictures is good to land === 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 [18:29] EdwinGrubbs, thanks. [18:58] EdwinGrubbs, could you please review https://code.edge.launchpad.net/~abentley/launchpad/revert-enums/+merge/23416 ? === 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 [19:21] abentley: I can do that after lunch [19:21] EdwinGrubbs, thanks. === 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 [20:15] 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:16] leonardr: looking [20:19] leonardr: what do you think of bin/test running both? [20:20] gary: bin/test picks up all the tests, not just the standalone ones [20:21] leonardr: oh I see. cool [20:24] 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:25] i can fix it up [20:25] ok cool [20:26] 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:27] gary: yes, definitely. it is creating a ServiceRoot object and parsing wadl [20:27] ok cool [20:44] leonardr: approved [20:44] great === 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 [23:26] 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] abentley: ^ might interest you === 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 [23:27] james_w: I'll look at that. [23:27] thanks [23:27] tiny branch [23:28] james_w: I thought pipelines were replacing looms. [23:28] nope [23:28] not at this point anyway [23:45] james_w: s/pipline/pipeline/ [23:45] james_w: r=me [23:45] thanks === 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