= ReviewerMeeting20090415 = == summary == * stub graduates == log == {{{ Apr 15 10:00:21 #startmeeting Apr 15 10:00:22 Meeting started at 09:00. The chair is barry. Apr 15 10:00:22 Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] Apr 15 10:00:45 hello everyone and welcome to this week's ameu reviewers meeting. who's here today? Apr 15 10:00:54 me Apr 15 10:00:54 me Apr 15 10:00:55 me Apr 15 10:01:21 me Apr 15 10:02:01 me Apr 15 10:02:13 me Apr 15 10:02:18 adeuring, bac ping Apr 15 10:02:20 * flacoste (n=francis@canonical/launchpad/flacoste) has joined #launchpad-meeting Apr 15 10:02:23 me Apr 15 10:02:26 me Apr 15 10:02:27 bigjools, BjornT ping Apr 15 10:02:34 danilo_: ping Apr 15 10:02:35 me, sorry for the delay Apr 15 10:02:38 me Apr 15 10:02:46 gmb: ping Apr 15 10:03:04 rockstar, salgado, sinzui ping Apr 15 10:03:16 me Apr 15 10:03:22 [TOPIC] agenda Apr 15 10:03:24 New Topic: agenda Apr 15 10:03:37 * Roll call Apr 15 10:03:37 * Action items Apr 15 10:03:37 * Mentoring update Apr 15 10:03:37 * Peanut gallery (anything not on the agenda) Apr 15 10:03:38 mo Apr 15 10:03:41 me Apr 15 10:03:47 pretty light day today (planned at least) Apr 15 10:04:02 [TOPIC] action items Apr 15 10:04:03 New Topic: action items Apr 15 10:04:07 me Apr 15 10:04:11 * danilo to look into storm/sqlobject result set compatibility Apr 15 10:04:32 is danilo_ here today? flacoste do you have any status on this? Apr 15 10:04:38 me Apr 15 10:04:40 danilo? Apr 15 10:04:45 that was allenap Apr 15 10:04:54 backporting the patch he landed on storm trunk Apr 15 10:05:03 I forgot :( Apr 15 10:05:09 flacoste: oops! Apr 15 10:05:16 me Apr 15 10:05:23 Please assign to me and I'll try and do it this time :) Apr 15 10:05:43 allenap: done, thanks Apr 15 10:05:52 * gary_poster will check to see if there's a bug open for adding a hook to `bzr send`, and submit one if there isn't Apr 15 10:06:04 no sorry Apr 15 10:06:08 will put back on my list Apr 15 10:06:13 gary_poster: cool, thanks Apr 15 10:06:21 * flacoste to work on API reviewer cheat sheet Apr 15 10:06:24 gary_poster, barry, abentley has a branch that he's been working on the forever. Apr 15 10:06:41 rockstar: ok cool, I'll ask him about it Apr 15 10:06:44 barry: no progress since last week Apr 15 10:06:49 np Apr 15 10:06:53 barry: keep it up, i like this weekly "i suck" reminder :-) Apr 15 10:06:57 :-D Apr 15 10:06:58 :-) Apr 15 10:07:02 good for keeping the ego in leash Apr 15 10:07:14 gary_poster: I've done the support for specifying a body when sending, but not adding the hook. Apr 15 10:07:43 abentley: cool. I'm guessing it's not on your immediate plate to finish it up? Apr 15 10:07:57 gary_poster: Right. Apr 15 10:08:01 understood Apr 15 10:08:09 thanks Apr 15 10:08:34 [TOPIC] mentoring update Apr 15 10:08:35 New Topic: mentoring update Apr 15 10:08:42 barry: Did you get anywhere with adding body support to claws? Apr 15 10:08:42 abentley: do you happen to know if there's a bug for this, and what it might be, if so? Apr 15 10:08:55 gary_poster: I'm not aware of a bug for this. Apr 15 10:08:55 abentley: i didn't :( Apr 15 10:09:04 abentley: ok cool. Apr 15 10:09:30 barry: one to-do item scratched off ;-) ...but maybe needs to be replaced with another. Apr 15 10:09:50 gary_poster: i'll cross it off and you can let me know if you want another Apr 15 10:09:58 barry: cool thanks Apr 15 10:10:13 am i right that only stub is still being mentored? Apr 15 10:11:16 i should ping a few folks to see if they're ready to be mentats Apr 15 10:11:33 i don't think there's many folks left who aren't revewiers Apr 15 10:11:58 anyway... Apr 15 10:12:02 [TOPIC] peanut gallery Apr 15 10:12:03 New Topic: peanut gallery Apr 15 10:12:14 do you guys have anything not on the agenda? Apr 15 10:12:41 I am wondering if we have a policy on redundant assertions? Apr 15 10:12:54 e.g. if foo; else: assert not foo Apr 15 10:13:12 abentley: i thought the policy is that we always do this Apr 15 10:13:22 but maybe it's not documented anywhere? Apr 15 10:13:28 intellectronica: we always have an else clause if there's an elif Apr 15 10:13:40 but if there's no elif we don't need an else+assert Apr 15 10:13:41 oh, right Apr 15 10:13:42 Why would we *always* do that? Apr 15 10:13:58 abentley's example seems to be different than what barry showed intellectronica Apr 15 10:14:12 abentley: doesn't make sense to me Apr 15 10:14:23 Yea, the else+assert seems a little silly, as though assert would catch something the if would not. Apr 15 10:14:26 abentley: do you have an example in our code? Apr 15 10:14:41 BjornT: It was something I reviewed on Monday. Apr 15 10:14:44 yes, sorry, i got confused. if there's only one if there's probably no need to do this, unless it's imprtant that the code fails at this point Apr 15 10:15:08 https://code.edge.launchpad.net/~salgado/launchpad/bug-358498/+merge/5496 Apr 15 10:16:01 abentley: the one on line 16 can just as well be raise AssertionError(...) Apr 15 10:16:19 in this specific case I wanted the assertion because the 'elif' block should be removed soon, at which point the assertion will become non-redundant Apr 15 10:16:21 and that does fall within our guidelines Apr 15 10:16:24 barry: It's unreacable code. Apr 15 10:16:44 abentley: proven by the assertion? Apr 15 10:16:51 barry: It can equally well be raise IAmSuperman Apr 15 10:17:33 abentley: the one on line 13 seems okay, but should have a message Apr 15 10:17:48 barry: It just seems like the wrong use of asserts-- proving that Python isn't buggy. Apr 15 10:17:50 abentley: ok. i would replace that assert with a raise AssertionError Apr 15 10:17:57 salgado: you're saying the whole 8-14 block will be removed eventually? Apr 15 10:18:15 abentley: sorry, i was talk about the one in the else clause (first) Apr 15 10:18:30 i.e. line 16 is fine Apr 15 10:18:54 line 13, yeah, i suppose. doesn't bother me too much either way Apr 15 10:19:00 barry: I thought asserts were for proving your own state wasn't buggy. Apr 15 10:19:31 abentley: yes, or that the state you're assuming to be the case actually is the case Apr 15 10:19:33 Anyhow, I'll continue to say "I think this is redundant, but keep it if you like." Apr 15 10:19:45 abentley: +1, for the line 13 assert Apr 15 10:19:59 oh, yeah, line 16 isn't a change Apr 15 10:20:06 but still. or whatever. Apr 15 10:20:16 * salgado_ (n=salgado@189-46-175-24.dsl.telesp.net.br) has joined #launchpad-meeting Apr 15 10:20:27 barry: line 13 can't be reached because of line 6. Apr 15 10:20:27 abentley: thanks Apr 15 10:20:34 abentley: yep Apr 15 10:20:56 * salgado has quit (Nick collision from services.) Apr 15 10:20:58 * salgado_ is now known as salgado Apr 15 10:21:15 * barry agrees with abentley when he says: Anyhow, I'll continue to say "I think this is redundant, but keep it if you like." Apr 15 10:21:20 oh, we're talking about line 13. yes, that one should be removed Apr 15 10:22:01 well, yes, but salgado said that he will soon reorganize the code, so leaving this as a"reminder" is OK, IMHO Apr 15 10:22:35 agreed. maybe the assertion message (which salgado will add ) should state something to that effect? Apr 15 10:23:14 maybe. :) Apr 15 10:23:46 abentley: thanks for bringing this up Apr 15 10:23:53 anything more on this? Apr 15 10:23:56 barry: np Apr 15 10:24:24 does anybody else have anything not on the agenda? Apr 15 10:25:24 5 Apr 15 10:25:32 4 Apr 15 10:25:33 Happy Tax Day! Apr 15 10:25:49 rockstar: "happy"?! Apr 15 10:25:50 3 Apr 15 10:25:56 2 Apr 15 10:26:01 1 Apr 15 10:26:05 #endmeeting }}} {{{ Apr 15 18:32:12 #startmeeting Apr 15 18:32:13 Meeting started at 17:32. The chair is barry. Apr 15 18:32:13 Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] Apr 15 18:32:24 hi jml, thumper. are you here? Apr 15 18:32:26 here Apr 15 18:32:34 barry: like you wouldn't believe. Apr 15 18:32:59 not that i have a big agenda Apr 15 18:33:11 [TOPIC] agenda Apr 15 18:33:12 New Topic: agenda Apr 15 18:33:18 * Roll call Apr 15 18:33:18 * Action items Apr 15 18:33:18 * Mentoring update Apr 15 18:33:18 * Peanut gallery (anything not on the agenda) Apr 15 18:33:24 [TOPIC] action items Apr 15 18:33:25 New Topic: action items Apr 15 18:33:38 thumper: just one for you, an old one i think Apr 15 18:33:40 * thumper to open bug on `webservice` pagetests globs problem Apr 15 18:33:53 hmm Apr 15 18:33:57 I don't recall doing it Apr 15 18:34:20 do you still want to? :) Apr 15 18:34:43 I should do it Apr 15 18:34:56 i can keep it on the agenda Apr 15 18:35:44 [TOPIC] * Peanut gallery (anything not on the agenda) Apr 15 18:35:46 New Topic: * Peanut gallery (anything not on the agenda) Apr 15 18:35:50 you guys have anything? Apr 15 18:36:01 bug 362032 Apr 15 18:36:02 Launchpad bug 362032 in launchpad "webservice glob is an admin user" [Undecided,New] https://launchpad.net/bugs/362032 Apr 15 18:36:26 I don't really have anything Apr 15 18:36:32 thumper: thanks Apr 15 18:36:41 jml? Apr 15 18:38:37 jml: i guess not Apr 15 18:38:48 i have one issue that came up today in a review: Apr 15 18:38:56 * don't return unwrapped objects from view methods, barry and gary ("the fightin' arries!") Apr 15 18:39:13 back Apr 15 18:39:23 computer crapped out Apr 15 18:39:26 i agree with the principle. gary wants to go further and prevent view modules from importing removeSecurityProxy Apr 15 18:39:30 jml: no worries Apr 15 18:39:33 barry: what do you mean? Apr 15 18:39:33 what did I miss? Apr 15 18:39:46 jml: not much. do you have any agenda items? Apr 15 18:39:52 I disagree with not allowing removeSecurityProxy at all Apr 15 18:40:05 but it's uses should be well defined Apr 15 18:40:05 just one re mentoring. Apr 15 18:40:10 thumper: can you state why? Apr 15 18:40:16 jml: we'll come back to that Apr 15 18:40:31 not off the top of my head Apr 15 18:40:38 I'd have to look at the situations where we use it Apr 15 18:40:41 and why we did it that way Apr 15 18:41:09 thumper: think about it, you don't have to answer now Apr 15 18:41:11 barry: so, I'm not against adding a rule wrt removeSecurityProxy but... Apr 15 18:41:23 it's not going to help that much :) Apr 15 18:41:36 why not? Apr 15 18:41:44 two reasons Apr 15 18:42:13 for a start, it's actually really easy to return non-wrapped objects, particularly if they aren't db objects Apr 15 18:42:34 even without rSP Apr 15 18:42:51 the issue goes more deeply into the model code Apr 15 18:43:13 e.g. up until recently, all SourcePackage objects had no security proxy at all. Apr 15 18:43:44 second, the purpose of the importfascist is unclear Apr 15 18:44:47 and so a prohibition there will last exactly as long as it takes for people to need removeSecurityProxy in view code and to forget exactly why it is that we don't allow it. Apr 15 18:45:31 btw, fwiw, i think rSP in view code is fine under the right circumstances Apr 15 18:46:15 so, what are the reasons for not returning unwrapped objects from views? Apr 15 18:47:09 I'm actually happy for "not returning unwrapped objects from views" Apr 15 18:47:17 keep the unwrapped objects in a defined scope Apr 15 18:47:18 jml: it's a vague (to me) feeling that we could leak private data Apr 15 18:47:41 however I'm against "don't allow the unwrapping of objects in views" Apr 15 18:48:39 barry: I think the real problem there is that it's difficult to audit any given page. Apr 15 18:49:12 jml: agreed. thumper agreed Apr 15 18:49:20 barry: in some ways, rSP makes it easier to audit :) Apr 15 18:49:47 i'm definitely against unwrapping in model code. so where else would you do it? i don't like having to add an artificial layer in there Apr 15 18:49:52 jml: true Apr 15 18:50:16 * bigjools has quit (Read error: 60 (Operation timed out)) Apr 15 18:50:28 so, I guess I'd say "whatever". We've already got quite a few well-intentioned for-your-own Apr 15 18:50:46 -good rules. One more won't break _this_ camel. Apr 15 18:51:16 jml: cool, this is an interesting discussion anyway. we can move on and we'll bring it up at the next ameu Apr 15 18:51:29 [TOPIC] mentoring update Apr 15 18:51:30 New Topic: mentoring update Apr 15 18:51:34 o hai Apr 15 18:52:02 stub's doing a great job, and I see no reason not to promote him. Apr 15 18:52:39 beauty. i'll make it official, thanks Apr 15 18:52:42 np Apr 15 18:52:48 i think he's the last mentat atm Apr 15 18:53:11 well, that's all i've got. anything else from you two? Apr 15 18:53:18 nope Apr 15 18:53:25 I'm cool. Apr 15 18:53:39 #endmeeting }}}