[05:23] <lifeless> LFR https://code.edge.launchpad.net/~lifeless/launchpad/registry/+merge/35774
[05:51] <jtv> allenap: I've got a modest branch up for review as well, if you have time: https://code.launchpad.net/~jtv/launchpad/bug-594220/+merge/35770
[05:51] <jtv> (bit early for you though innit?)
[06:03] <jtv> StevenK: is allenap really still reviewing your branch, or is the topic outdated?
[06:03] <lifeless> stale topic
[06:04] <jtv> so there
[06:04] <jtv> and this system has gotten too weird for me; I'll reboot
[07:33] <jtv> hi henninge!
[07:34] <henninge> Hi jtv!
[07:35]  * StevenK gets more tempted to write a review helper bot that will manage the topic and MPs
[07:35] <StevenK> I could use PoCo::IRC just to annoy people too :-P
[09:13] <allenap> jtv: Sorry, I forgot to take my name out of the topic :-/
[09:13] <jtv> allenap: we figured it out, no worries :)
[09:13] <StevenK> allenap: And you didn't actually review my branch at all!
[09:13] <allenap> StevenK: I was a whisker from finishing your branch review last night but I had to go. I'll finish it now.
[09:13] <StevenK> Hehe
[09:13] <StevenK> allenap: It's perfectly fine, thanks. :-)
[09:20] <allenap> StevenK: Done, needs (a bit of) fixing :)
[09:21] <noodles775> henninge: When you've time: http://launchpadlibrarian.net/55794871/635005-updated.png
[09:22] <noodles775> There are two things I didn't do (indentation, download link) and one extra thing (binary descriptions).
[09:23] <noodles775> henninge: I did try the indentation, but it seems that we explicitly set ul's and dl's to margin/padding 0px; so I think that's a site-wide decision that I didn't want to touch as part of this branch.
[09:23] <noodles775> Regarding the download link, .... actually, I'll send the MP email, so your mentor can also read.
[09:24] <noodles775> henninge: hrm, I'd already mentioned the link on the previous email. So, nothing more to say :)
[09:24] <StevenK> allenap: I can't use the ZCML for auth, sadly. Using soyuz-team is a stop-gap first-step. When we want other people to be able to use it, it will switch to require the user being a driver of the distribution.
[09:24] <allenap> StevenK: Can you explain why?
[09:25] <StevenK> allenap: Why we require only soyuz-team, or the user being a driver?
[09:26] <allenap> StevenK: No, why not ZCML? Is there a technical reason? I'm interested as much as anything.
[09:27] <StevenK> allenap: I didn't think you can use ZCML if it requires something more complex than the permission the user has.
[09:28] <allenap> StevenK: Permissions are given to user by a security adapter based upon the context. So, for an IDistroSeries, launchpad.Edit can be granted if the user is in ~soyuz-team. Take a look at some of the adapters in canonical.launchpad.security.
[09:29] <StevenK> allenap: Ah, sorry -- I was skipping ahead. ~soyuz-team is not the final solution. I don't think I can do it if it's dependant on what distribution the belongs to
[09:30] <StevenK> Sigh, words are not my friend today
[09:31] <StevenK> allenap: I have a stand-up starting, well, when bigjools notices what time it is. Would you like a 5 minute chat about it after that?
[09:31] <allenap> StevenK: :) Sure, ping me when you've got time.
[09:33] <henninge> noodles775: care to paste the mp url again, please? ;-)
[09:33] <noodles775> henninge: https://code.launchpad.net/~michael.nelson/launchpad/635005-difference-details-2/+merge/35640
[09:43] <StevenK> allenap: Ready when you are, are you mumble-enabled?
[10:04] <henninge> noodles775: approved* ;-)
[10:04] <noodles775> Thanks henninge
[10:06]  * henninge needs to reboot computer
[10:16] <bigjools> happy birthday henninge
[10:37] <jtv> hey abel, stealth review—thanks!
[12:23] <noodles775> Hi adeuring, I've got a 797liner if you've time. Just let me know if you'd prefer I split it up:
[12:23] <noodles775> https://code.edge.launchpad.net/~michael.nelson/launchpad/635005-difference-details-2/+merge/35640
[12:23] <adeuring> noodles775: nah, I'll look at the whole branch
[12:24] <noodles775> adeuring: great, thanks. I'll just update the description explaining what the bloat is.
[12:27] <noodles775> adeuring: description updated.
[12:33] <noodles775> adeuring: also (sorry), I've just realised that I no-longer need the generic CommentView... I'll remove it.
[12:34] <adeuring> noodles775: no problem -- i just realized that I hadn't have lunch yet, will make a lunch break before I start to look at your branch ;)
[12:34] <noodles775> Great, enjoy :)
[13:57] <bac> hi adeuring
[13:57] <bac> can i add one to your queue?
[13:58] <bac> adeuring:  https://code.edge.launchpad.net/~bac/launchpad/lp-db-setup/+merge/35822
[13:58] <gmb> adeuring, bac I have a very large (1300 lines) lazr-js branch that needs review. This adds a new Wizard widget. Can either of you guys review it?
[13:59] <gmb> (A lot of those lines are '},' of course ;))
[13:59] <bac> gmb: i will but you may still want to run it by a JS expert
[14:00] <adeuring> bac: sure, let me first finish noodles775 review
[14:00] <bac> adeuring:  no rush
[14:01] <gmb> bac, Eyefankyoo. I'll see if I can find an eggspurt.
[14:02] <gmb> bac, https://code.edge.launchpad.net/~gmb/lazr-js/wizard-widget/+merge/35827, FTR.
[14:12] <gmb> mars, Around?
[14:30] <adeuring> noodles775: r=me
[14:30] <noodles775> Thanks adeuring
[14:31] <noodles775> rockstar: Hi! When you get around to mentoring henninge's UI review, would you mind also taking a brief look at the code and make sure I'm not abusing the lp.services.comments in a way you guys didn't intend?
[14:32] <noodles775> https://code.edge.launchpad.net/~michael.nelson/launchpad/635005-difference-details-2/+merge/35640
[14:45] <mars> gmb, pong, what's up?
[14:47] <gmb> mars, I have a large (~1300-line) lazr-js branch that adds a Wizard widget (building upon work that rockstar had already done, but which hadn't landed). bac is reviewing it but he suggested that I should find a JS expert to take a look, too. Your name was top of my list. Do you have time to take a look?
[14:48] <mars> gmb, unfortunately I do not have time today.
[14:49] <mars> gmb, Paul and Edwin are two other good candidates
[14:49] <gmb> mars, Okay, no worries. I'll ask someone else.
[14:49] <gmb> Indeed.
[14:49] <gmb> So, rockstar, are you free to look at how I've mangled your widget?
[14:49] <gmb> ooh, that sounded wrong.
[14:49] <adeuring> bac: r=me
[14:49] <mars> lol
[14:53] <bac> adeuring:  thanks
[15:12] <bac> gmb:  dumb question - should the examples work if simply loaded into ff
[15:12] <gmb> bac, Yes. And that said, let me check that I didn't break them, 'cos I've just realised that I might have.
[15:13] <bac> gmb:  yeah, none of them do squat for me
[15:13] <bac> gmb:  could you have messed them all up?
[15:14] <gmb> bac, Works for me (bar one small tweak; fixed and pushed).
[15:14] <gmb> bac, Did you make build in the root of the branch?
[15:15] <bac> gmb: gah
[15:15] <gmb> Hah
[15:16] <gmb> bac, It took me a while to figure that out, too.
[15:16] <gmb> JS shouldn't be this hard. YUI, I'm looking at you, here.
[15:23] <bac> gmb: ok, so i got it built and most JS examples are working now.  your wizard doesn't seem to, though, as it just shows Step One
[15:24] <bac> gmb: am i doing something else wrong?
[15:24] <gmb> bac, So, the 'next' button doesn't work, then?
[15:25] <gmb> Note that I've spent more time on tests than on the example, so if it's lacking, that's why.
[15:25] <bac> gmb: there is no next button.  just a "re-open the wizard" link
[15:25] <gmb> ...
[15:26] <gmb> bac, That's very, very odd.
[15:27] <gmb> Can you take a look at Firebug, see if there are any errors?
[15:28] <gmb> bac, In fact, I'd go so far as to say the example should be completely ignored, because it's rubbish compared to other examples.
[15:31] <bac> gmb: if i load the file i do not get an overlay
[15:31] <bac> but if i open firebug and then load the file then the overlay appears
[15:31]  * gmb tries to reproduce
[15:32] <rockstar> gmb, I'm happy to look at it if you don't have a reviewer yet.
[15:34] <gmb> rockstar, bac's looking but having some issues; I'd appreciate your eye on the code itself (ignore the example, though, it's bobbins).
[15:35] <gmb> Huh.
[15:35] <bac> rockstar: i will do a full review per our discussion a few weeks ago that everyone should do JS reviews
[15:35] <gmb> bac, I can reproduce that.
[15:35] <gmb> bac, Try chromium. It works fine there.
[15:35] <bac> :)
[15:35] <rockstar> bac, great, thanks.
[15:35] <gmb> Meanwhile, I'll try to hammer firefox into submission.
[15:36] <bac> rockstar: but i suggested to gmb that he get a JS expert to look at it too
[15:36] <rockstar> bac, yeah, I'd at least like to look at it from a "is it YUI-y" standpoint.
[15:36] <gmb> bac, I know why it happens, I think. Hang on.
[15:39] <rockstar> gmb, before you land it (and after bac reviews it) I'd love to take a whack at reviewing it as well.
[15:39] <gmb> bac, Pull and rebuild. Fixed it by a cunning s/console/Y/g
[15:39] <gmb> rockstar, Please do. I'm sure it's littered with horrors.
[15:39] <gmb> And things I've forgotten to add tests for.
[15:39]  * gmb hates doing the tests second.
[15:39] <rockstar> gmb, yeah, but I think testing this kind of interaction is hard without huge things like windmill.
[15:41] <gmb> rockstar, Agreed.
[15:43] <bac> gmb: works brilliantly now
[15:44] <gmb> bac, Well, it works 100% better than it did, anyway :)
[16:58] <bac> gmb: your testing is very thorough and easy to follow
[17:00] <bac> gmb: i am confused by test_wizard_needs_steps
[17:01] <gmb> bac, How so?
[17:01] <bac> gmb: in it you seem to be instantiating with no steps, which is a no-no, but assert that it instaniated properly
[17:01] <bac> gmb: am i misreading it?
[17:01] <gmb> bac, Ah, JS testing FTFL.
[17:02] <gmb> bac, See the _should definitions
[17:02] <bac> yeah, i did but clearly nothing clicked
[17:02] <gmb> bac, test_wizard_needs_steps is understood to have passed if it raises an error.
[17:03] <gmb> bac, I'll add a comment to make this clear, but it is beyond horrible that we can't do Assert.throws() or some such.
[17:03] <bac> so the assert does fail
[17:03] <bac> and that failure is noted in the _should
[17:04] <bac> so by failing it passes
[17:04] <gmb> bac, Yes
[17:04]  * bac cries
[17:04] <bac> yes, i knew that at one time
[17:04] <bac> but my brain purged it as it was jibberish
[17:04] <gmb> Yeah.
[17:07] <bac> gmb: in  _renderUIWizard: you check to see that there are some steps
[17:08] <bac> gmb: is that necessary since you did the check at instantiation?
[17:08] <gmb> bac, Oops, no. I'll remove that.
[17:13] <bac> gmb r=bac but i hope rockstar has time to have a look
[17:14] <rockstar> bac, I will MAKE time.
[17:14] <gmb> bac, ta
[17:28] <gmb> bac, It should be noted FTR that rockstar did many of the changes for which you've given positive comments. :). However, sadly, I can't blame any problems on him.
[17:28] <rockstar> gmb, the fact that you pulled it from the railroad tracks before the train came should also be noted.
[17:29] <rockstar> gmb, where is this proposal?
[17:29] <gmb> rockstar, Oh yes, that might help you... https://code.launchpad.net/~gmb/lazr-js/wizard-widget/+merge/35827
[17:39] <allenap> bac: Could you sanity check this please? http://paste.ubuntu.com/495399/ - it's a change suggested by lifeless in https://code.edge.launchpad.net/~allenap/launchpad/bugsubscription-to-storm/+merge/35537
[17:41] <allenap> bac: Argh, I've got to go! Don't worry about it. Thanks anyway.
[18:42] <EdwinGrubbs> bac: I have a 950 line branch. Do you want to review it? It will still need a follow-up branch to add the cronjob before it lands.
[19:03] <bac> EdwinGrubbs: i'll be happy to review it
[19:07] <bac> EdwinGrubbs let me know when you've submitted a MP
[19:23] <EdwinGrubbs> bac: here it is: https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-615654-queue-addmember-emails/+merge/35862
[19:40] <EdwinGrubbs> bac: I'm headed to lunch.
[19:40] <bac> EdwinGrubbs ok
[19:40] <bac> don't go to that place where they yell at you
[19:43] <EdwinGrubbs> I wonder how they would do as code reviewers.
[21:34] <EdwinGrubbs> bac: thanks for the review. I'll try to get the reply done shortly. I tried going to Starbucks after lunch, but freenode had banned its IP address. Stupid caffeinated hackers.
[21:34] <bac> really?  hadn't heard of that before
[21:34] <bac> EdwinGrubbs: i would suggest bip but it's a pita
[21:48] <sinzui> bac ping
[21:48] <bac> hi sinzui
[21:48] <sinzui> bac: I am preparing a migration branch for mailman. Will you have time to review it?
[21:49] <bac> sinzui: how big?
[21:50] <sinzui> 417, 30 lines are not mechanical--two tests were broken by recent changes
[21:50] <bac> i can definitely do it tonight
[21:51] <bac> sinzui: around 6 we're leaving but i'm can do it in the car and send it when we arrive
[21:51] <sinzui> it is very boring. I was just disappointed to see the tests were broken in july/August
[21:52] <bac> sinzui: they were broken and then disabled?
[21:52] <bac> sinzui: or are they only run on demand?
[21:53] <sinzui> latter --layer=Mailman and they only pass on repeated tries
[21:53] <bac> outta sight...gonna break
[21:54] <sinzui> I am so angry with these tests. One that is broken does not need to be on the mailman layer.
[21:54] <sinzui> I do not know where to begin fixing these tests
[21:55] <bac> so when do you think the branch will be ready?
[21:55] <sinzui> 5 minutes
[21:55] <bac> oh, sweet
[22:03] <sinzui> bac: I am still waiting for the diff to appear: https://code.edge.launchpad.net/~sinzui/launchpad/discard-list-spam-0/+merge/35885
[22:03]  * bac makes my own diff
[22:20] <bac> sinzui: wow!  that was boring
[22:20] <bac> r=me
[22:20] <sinzui> running the tests is much more boring
[22:21] <sinzui> I need caffeine, Guinness, and maybe LSD to find a way to make this testable.
[22:23] <EdwinGrubbs>  bac: do you know if there is a helper function like assert that raises ValueError> My single line asserts turned into 4-line if-statements.
[22:23] <EdwinGrubbs> s/>/?/
[22:24] <bac> EdwinGrubbs: i don't know.
[22:26] <bac> EdwinGrubbs: it sounds like a good helper, though
[22:26] <bac> sorry for the clutter
[22:32] <EdwinGrubbs> sinzui: I was looking at volunteering with Junior Achievement. Apparently, it would take a couple hours a week for eight weeks. Is it ok if I shift my schedule on those days, and is it ok if I miss the standup on those days?
[22:32] <EdwinGrubbs> sinzui: oops, this should go in the lp-registy channel.
[22:48] <benji> EdwinGrubbs: I'd have to look at the style guide to see if it's allowed, but removing the newline after the "if" seems reasonable:  "if not value_is_good: raise ValueError('you suck')"
[22:57] <EdwinGrubbs> benji: well, the main reason that the ValueError is really long is that I was making a very clear error message. I guess I really only need to include the bad value, since the variable name and type can be found in the source code.
[22:58] <benji> EdwinGrubbs: I suggest a message like "foo_var must be a positive integer, got %r instead" % (foo_var,)
[23:02] <EdwinGrubbs> providing that long error message over and over again seems like a perfect reason for a helper function, but providing the variable and the variable name either means adding a redundant looking arg, using vars(), or crawling up the stack. bleh.
[23:05]  * benji goes afk
[23:21] <lifeless> benji: EdwinGrubbs: FWIW: practicality over purity.
[23:21] <lifeless> our style guide is a *guide* not a straightjacket.