[04:23] <mwhudson> sigh
[04:23] <mwhudson> anyone want to review a branch?
[04:51] <mwhudson> https://code.edge.launchpad.net/~mwhudson/launchpad/puller-import-hack/+merge/23370
[10:11] <jml> hi
[10:11] <jml> would someone please review https://code.edge.launchpad.net/~jml/launchpad/extract-ssh-server/+merge/23350
[10:12] <jml> it's a small patch that shouldn't change any behaviour
[11:50] <maxb> I would like to submit this rather boring and small branch for review: https://code.edge.launchpad.net/~maxb/launchpad-cscvs/use-hashlib
[12:57] <danilos> allenap, hi, got a review ready for you :) https://code.edge.launchpad.net/~danilo/launchpad/bug-562450/+merge/23383 (diff is being regenerated)
[13:09] <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:53] <intellectronica> allenap: interested in a branch for review?
[13:54] <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:55] <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:56] <intellectronica> deryck: sure, i'll do the ui review (with sinzui mentoring)
[13:57] <wgrant> Two of the branches are trivial, though...
[14:00] <jml> I've just reviewed & approved maxb's branch
[14:01] <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:02]  * 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:03] <jml> groan
[14:03] <jml> it's not in 2a format
[14:03] <deryck> intellectronica, got it.  thanks.
[14:04] <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:05] <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:06] <kfogel> deryck: that was coincidence -- my client was off until a second ago.
[14:06] <kfogel> deryck: what's up?
[14:07] <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:08] <intellectronica> kfogel: it's easy to be a victim in the blame game when your irc client is off ;)
[14:09] <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:10] <intellectronica> ot1h it fits the template of all other lines, otoh it reads a bit funny
[14:10] <deryck> intellectronica, ok.  any suggestions?
[14:11] <intellectronica> "N bugs in progress" is the obvious alternative, but I don't know if it's actually better
[14:13] <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:15] <deryck> intellectronica, concerning your MP.... does the change from security_related to setSecurityRelated have any impact on the js widget for security setting?
[14:17] <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:18] <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:19] <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:20] <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:21] <jml> maxb: my pleasure
[14:21] <deryck> intellectronica, yup.  looks good to me.  r=me.
[14:21] <intellectronica> deryck: thanks!
[15:19] <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:28] <maxb> Thanks noodles775. Will you land it?
[15:29] <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:35] <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:36] <wgrant> Not pre-Lucid, since we don't have 2.6 on Hardy.
[15:36] <wgrant> But it's not far.
[15:37] <sinzui> deryck, did my suggestion terrify you?
[15:37] <deryck> sinzui, haven't looked yet, sorry.  let me look....
[15:39] <deryck> sinzui, I like your suggestion and am fine to do that.
[15:49] <daniloff> allenap, hi, did you get any chance to restart reviewing?
[15:50] <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:51] <EdwinGrubbs> daniloff: I'll be able to start on that in a few minutes.
[15:56] <abentley> allenap, EdwinGrubbs could you please review https://code.edge.launchpad.net/~abentley/launchpad/recipe-security/+merge/23357 ?
[15:58] <allenap> (He asked me a while back, sorry, forgot to update the topic.)
[16:09] <EdwinGrubbs> abentley: sure
[16:09] <abentley> EdwinGrubbs, thanks.
[16:18] <allenap> adeuring: Is there going to be another branch (or two) to add a date_expired field to IBugTask?
[16:19] <adeuring> allenap: argh... i forgot completely about that...
[16:19] <adeuring> yes, there _should_ be a branch...
[16:20] <allenap> adeuring: I think the bug number in the branch name is wrong.
[16:22] <adeuring> allenap: right...
[16:24] <allenap> adeuring: Line 75, could you login(product_owner) to avoid the removeSecurityProxy() call?
[16:25] <allenap> adeuring: Or even login_person(product_owner)
[16:26] <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:29] <allenap> adeuring: Perhaps the removeSecurityProxy() hack could be put in LaunchpadObjectFactory.makeProduct(..., bug_supervisor=None).
[16:29] <adeuring> allenap: right, good idea!
[16:30] <leonardr> allenap, edwin: pushing https://code.edge.launchpad.net/~leonardr/lazr.restful/561521/+merge/23404 onto the queue
[16:35] <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:36] <adeuring> allenap: right
[16:37] <allenap> adeuring: 136s/Epired/Expired/
[16:37] <adeuring> allenap: fixed
[16:39] <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:40] <adeuring> allenap: I did not find anything like that...
[16:40] <adeuring> that's why I added it ;)
[16:40] <allenap> adeuring: Heh :)
[16:53] <allenap> wgrant: Want me to land lp:~wgrant/launchpad/always-use-sourcepackagename.name for you?
[16:56] <allenap> wgrant: I've sent it off to land via ec2.
[17:15] <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:16] <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:17] <allenap> sinzui: Okay. Also, can_show_portlet() can return True if there are sourcepackages even when user is None. Is that right?
[17:18] <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:19] <sinzui> There can be a list like this: https://edge.launchpad.net/gdp
[17:20] <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:21] <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:22] <allenap> sinzui: Okay, that makes sense, thanks.
[17:22]  * sinzui has to rethink the model to support Not Packaged
[17:25] <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:26] <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:27] <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:28] <sinzui> james_w, you meana source package name cannot contain underscores?
[17:29] <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:30] <sinzui> ah yes, I recall someone wanted to create a gentoo package with an '/' in it
[17:31] <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:52] <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:53] <rockstar> sinzui, looking
[17:54] <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:55] <sinzui> I would say too much because I think I have seen 10 and I tuned out
[17:55] <sinzui> rockstar, 20
[17:56] <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:57] <sinzui> rockstar, no need to speculate: https://edge.launchpad.net/launchpad
[17:58] <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
[18:02] <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:06] <EdwinGrubbs> abentley: r=me
[18:14] <rockstar> sinzui, great.  With those changes, ui=me*
[18:15] <sinzui> rockstar, thanks. tests pass after the change
[18:15] <rockstar> sinzui, noodles still has to mentor me.
[18:16]  * 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:18] <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:19] <sinzui> deryck, that looks great
[18:19] <deryck> sinzui, cool, thanks.
[18:20] <sinzui> I think borders would make me think too much about the filters. your pictures is good to land
[18:29] <abentley> EdwinGrubbs, thanks.
[18:58] <abentley> EdwinGrubbs, could you please review https://code.edge.launchpad.net/~abentley/launchpad/revert-enums/+merge/23416 ?
[19:21] <EdwinGrubbs> abentley: I can do that after lunch
[19:21] <abentley> EdwinGrubbs, thanks.
[20:15] <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:16] <gary_poster> leonardr: looking
[20:19] <gary_poster> leonardr: what do you think of bin/test running both?
[20:20] <leonardr> gary: bin/test picks up all the tests, not just the standalone ones
[20:21] <gary_poster> leonardr: oh I see.  cool
[20:24] <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:25] <leonardr> i can fix it up
[20:25] <gary_poster> ok cool
[20:26] <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:27] <leonardr> gary: yes, definitely. it is creating a ServiceRoot object and parsing wadl
[20:27] <gary_poster> ok cool
[20:44] <gary_poster> leonardr: approved
[20:44] <leonardr> great
[23:26] <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:27] <EdwinGrubbs> james_w: I'll look at that.
[23:27] <james_w> thanks
[23:27] <james_w> tiny branch
[23:28] <EdwinGrubbs> james_w: I thought pipelines were replacing looms.
[23:28] <james_w> nope
[23:28] <james_w> not at this point anyway
[23:45] <EdwinGrubbs> james_w: s/pipline/pipeline/
[23:45] <EdwinGrubbs> james_w: r=me
[23:45] <james_w> thanks