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