= ReviewerMeeting20090128 = == summary == * barry was unable to eliminate the back-patching of schema types * storm api should be used for all new database classes * abentley to email ml and gustavo with suggestions for improving storm == logs == {{{ gary_poster joined the chat room. [10:00am] barry: #startmeeting [10:00am] MootBot: Meeting started at 09:00. The chair is barry. [10:00am] MootBot: Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [10:00am] barry: mootbot! [10:00am] barry: hello everyone and welcome to this week's ameu meeting. who's here today? [10:00am] bac: me [10:00am] EdwinGrubbs: me [10:00am] adeuring: me [10:00am] mars: me [10:00am] gmb: me [10:01am] rockstar: me [10:02am] barry: allenap: BjornT cprov gary_poster ping [10:02am] allenap: me [10:02am] gary_poster: me [10:02am] barry: intellectronica: ping? [10:02am] barry: salgado: ping [10:02am] salgado: me [10:03am] intellectronica: barry: i did apologise, no? [10:03am] barry: intellectronica: you did. sorry. i mssed that [10:03am] barry: intellectronica: thanks [10:03am] barry: well, i think it's a light day today so let's get to it [10:04am] barry: [TOPIC] agenda [10:04am] MootBot: New Topic: agenda [10:04am] barry: == Agenda == [10:04am] barry: * Roll call [10:04am] barry: * Peanut gallery (anything not on the agenda) [10:04am] barry: * Action items [10:04am] barry: * Mentoring update [10:04am] barry: i'm going to jump around a bit... [10:04am] barry: [TOPIC] action items [10:04am] MootBot: New Topic: action items [10:04am] barry: * barry to look into techniques for eliminating back-patching of schema types (avoiding circular imports) [10:04am] barry: i actually did look into this and tried a few things, but they all failed [10:05am] barry: i may do a very simple syntactic sugar branch for this but i might just drop this item [10:05am] barry: ping me if you're interested in the gory details :) [10:05am] barry: * barry to add `pretty()` functions to reviewers docs [10:05am] barry: not done [10:06am] barry: * flacoste to work on API reviewer cheat sheet [10:06am] mars: he's at the team leads sprint [10:06am] barry: i think flacoste probably didn't do this, and he's at tl sprint, so we'll just leave it [10:06am] • barry should type faster than mars [10:06am] barry: [TOPIC] * Peanut gallery (anything not on the agenda) [10:06am] MootBot: New Topic: * Peanut gallery (anything not on the agenda) [10:07am] barry: do you guys have anything not on the agenda? [10:07am] bac: barry: abentley had a proposed item [10:07am] rockstar: Yea, but I can't seem to find him. [10:07am] barry: * Should new classes be forbidden to use SQLObject compatibility layer, abentley [10:07am] barry: bac: thanks [10:08am] barry: looks like abentley is away on #launchpad-code [10:08am] rockstar: I think the answer is "Yes" [10:08am] barry: rockstar: i agree [10:08am] barry: bac: didn't you ping me yesterday about this? [10:08am] bac: abentley wrote a new db class that i reviewed yesterday. it was not using storm so i requested he modify it to use storm, as it is my understanding that new db work use the storm api. [10:09am] rockstar: We should deprecate the SQLObject compatibility layer altogether, and convert the old classes to Storm. [10:09am] bac: he objected on the grounds the storm api is not as nice as the compatibility layer. [10:09am] bac: i did not approve his branch pending the discussion he wanted to have here. [10:09am] • barry thinks it's nicer :) [10:09am] barry: rockstar: +1 [10:09am] bac: rockstar: that was my position [10:09am] rockstar: bac, this is true, but Storm is a Canonical project, so we should be dogfooding and making bug reports, etc. [10:10am] mars: barry, storm is nicer? or the compatibility layer? [10:10am] bac: rockstar: i totally agree. it's unfortunate that aaron is not here to state his case. [10:10am] • barry likes storm [10:10am] rockstar: I can't tell you how happy I am that the LP team is dogfooding reviews. It's allowed us to make all sorts of changes. [10:10am] gmb: +1 for Storm from me. [10:10am] bac: FTR i am +1 on requiring new work be storm api [10:10am] rockstar: +1 as well. I think there's real value in dogfooding. [10:10am] barry: i wish abentley was here too [10:10am] adeuring: +1 too [10:11am] barry: [VOTE] +1 to require all new classes to use the storm api [10:11am] MootBot: Please vote on: +1 to require all new classes to use the storm api. [10:11am] MootBot: Public votes can be registered by saying +1/-1/+0 in the channel, private votes by messaging the channel followed by +1/-1/+0 to MootBot [10:11am] MootBot: E.g. /msg MootBot +1 #launchpad-meeting [10:11am] barry: +1 [10:11am] MootBot: +1 received from barry. 1 for, 0 against. 0 have abstained. Count is now 1 [10:11am] adeuring: +1 [10:11am] MootBot: +1 received from adeuring. 2 for, 0 against. 0 have abstained. Count is now 2 [10:11am] rockstar: 1 [10:11am] gmb: +1 [10:11am] MootBot: +1 received from gmb. 3 for, 0 against. 0 have abstained. Count is now 3 [10:11am] bac: +1 [10:11am] MootBot: +1 received from bac. 4 for, 0 against. 0 have abstained. Count is now 4 [10:11am] allenap: +1 [10:11am] MootBot: +1 received from allenap. 5 for, 0 against. 0 have abstained. Count is now 5 [10:11am] gary_poster: +1 [10:11am] MootBot: +1 received from gary_poster. 6 for, 0 against. 0 have abstained. Count is now 6 [10:11am] salgado: +1 [10:11am] MootBot: +1 received from salgado. 7 for, 0 against. 0 have abstained. Count is now 7 [10:12am] barry: going once... [10:12am] barry: going twice... [10:12am] barry: sold! [10:12am] bac: barry, i'll take an action item to update wiki to reflect the policy. should it go in PSG? [10:13am] barry: unanimous! i'll post this to the ml and we can take objections there [10:13am] barry: [ACTION] barry to post storm requirement to ml [10:13am] MootBot: ACTION received: barry to post storm requirement to ml [10:13am] abentley joined the chat room. [10:13am] barry: abentley: hi! (almost) just in time :) [10:14am] abentley: me [10:14am] abentley: Sorry I'm late. Real Life stuff... [10:14am] barry: abentley: we were just discussing your agenda item. you have some 'splainin' to do :) [10:14am] barry: abentley: no worries [10:14am] abentley: Okay. [10:15am] abentley: Personally, I find the SQLObject compatibility shim a better API than the native Storm API. [10:15am] abentley: So I'd like to keep using it. [10:15am] abentley: But I've been told that official policy dictates that we use native Storm foo for new classes. [10:15am] abentley: So I wanted to bring it up for discussion. [10:15am] barry: abentley: what is it about the compatibility layer that you like better? [10:16am] abentley: barry: It doesn't require obtaining stores, it comes with get methods built-in, it comes with constructors built-in. [10:18am] barry: abentley: there's unanimous consensus (here, at least) that we should be dogfooding and improving storm, using it for all new classes [10:18am] mars: barry, I think abentley is arguing that the SQLObject API *is* an improvement on top of Storm [10:19am] allenap: Obtaining a store explicitly is a good pattern imho, because we must sometimes decide between master and slave. [10:19am] barry: mars: except that it's a horrible hack and i don't think it's even sqlobject-y any more [10:19am] abentley: allenap: Perhaps, but it's the wrong place to make that decision. [10:19am] mars: barry, ah [10:19am] al-maisan joined the chat room. [10:20am] abentley: allenap: You'd basically have to pass stores down a long call chain. [10:20am] barry: in the common case, it's one extra line [10:20am] barry: store = Store.of(self) [10:20am] • al-maisan apologizes for being late [10:20am] barry: okay, two. you have to import Store [10:20am] allenap: abentley: What barry said. [10:20am] rockstar: barry, but what about choosing between master and slave? [10:21am] barry: rockstar: i recall a discussion of this months ago. the outcome (iirc) is that you almost never really have to decide [10:21am] gmb: rockstar: It's still a one liner (imports notwithstanding). Slightly more complex [10:21am] EdwinGrubbs: rockstar: you normally let it automatically choose between master and slave based on whether it is a GET or POST request. [10:21am] abentley: You need to choose between master and slave at the call site, not in the implementation of a getter. [10:21am] barry: rockstar: and Store.of() will almost always DTRT [10:21am] rockstar: EdwinGrubbs, so it will figure out the Store automatically? [10:21am] abentley: barry: RIGHT! We are writing an extra line to no purpose at all. [10:22am] abentley: I think there are at least two import lines, maybe three, also. [10:22am] barry: abentley: well, you can also write it like: Store.of(self).find(...) [10:22am] abentley: barry: Not in a classmethod or static method. [10:22am] barry: abentley: sure, but those aren't the common case [10:22am] abentley: Which is where you'd put a getter. [10:23am] abentley: Those are the common case if you're writing a getter. [10:23am] EdwinGrubbs: rockstar: yes, if you have made a write (POST) in the last few minutes, it will use the master. You only need to choose the master manually, when you can't tell from the browser session that the user needs very up-to-date info. For example, registering a new user or logging in. [10:23am] barry: abentley: really? aren't getters usually instance methods? [10:23am] gmb: barry: They're instance methods for FooSet classes [10:23am] barry: gmb: right [10:23am] gmb: But we want to get rid of unnecessary *Sets, IIRC. [10:24am] barry: gmb: oh i see, you don't have a db object at that point. yeah [10:24am] abentley: gmb: I don't use FooSets. Classes are perfectly good utilities. [10:24am] gmb: abentley: ... which is why we want to get rid of unnecessary *Sets. [10:24am] abentley: Instead of class= you do component= and you're laughing. [10:26am] abentley: barry: I haven't been allocated time to improve the Storm API, and I'm not sure what kinds of changes are welcome. [10:26am] abentley: barry: It seems like the lack of constructors was a design choice, since all other ORMs do them. [10:27am] abentley: barry: So I don't think I'd be able to make Storm work as conveniently as the compatibility shim. [10:27am] barry: abentley: this all comes down to basic dogfooding rule #1. same as for bzr, merge proposals, lazr.config, and on-and-on [10:27am] mars: we have dogfooding rules? [10:28am] • barry recalls a rather lengthy ml discussion about dogfooding recently :) [10:28am] mars: ok, I'll look it up [10:29am] barry: mars: well, it's related to merge proposals and their emails [10:29am] mars: barry, ok, so what's rule #1 then? [10:29am] abentley: barry: You'll note that in that discussion I didn't say "we must not use ML", I said we should work toward not needed to use MLs. [10:30am] barry: mars: i'm making it up as i go, but "we should dogfood our own technology and improve it" [10:30am] barry: even if that means sending a message to gustavo and/or the storm list with suggestions [10:31am] barry: iow, it would be great for abentley to send an email explaining where he finds the native api to be less ideal so we can at least start a dialog about if/how to improve it [10:32am] barry: abentley: i'm not saying your complaints about storm are without merit, just saying we should improve storm instead of punting [10:33am] barry: that's just my opinion though so if there's disagreement about that, please speak up! [10:33am] allenap: barry: +1 on not punting. [10:33am] abentley: barry: How about we not punt and change the shim into an explicit storm-for-launchpad library? [10:34am] mars: abentley, you should look up the "Five Whys" from lean. It's a quality practice. I think you've asked why #1, but that leaves maybe four more :) [10:35am] barry: abentley: the goal should be to eventually get that merged into storm. unless there's compelling reasons to maintain our own shim i think the entire storm community should benefit from our experience [10:35am] abentley: barry: Sounds good to me. [10:36am] abentley: We can call it "mildstorm" [10:36am] mars: "turbulence" [10:36am] bac: i prefer squall [10:36am] mars: blizzard [10:36am] • barry looks out the window and suggests "blizzard" [10:36am] barry: mars: damn you again! :) [10:36am] abentley: barry: I hear ya. [10:37am] bac: just not "nor'easter" -- god i hate that [10:37am] barry: sounds good. abentley, start with an email to the ml. let's get the specific issues on the table. please cc gustavo [10:38am] abentley: Okay. [10:38am] barry: thanks [10:38am] bac: abentley: a side-by-side comparison of the class your new class from yesterday may be a good example [10:38am] abentley: I take it not using the shim *really is* official policy? [10:38am] bac: doh, not writing english today [10:38am] barry: [ACTION] abentley to email ml and gustavo with suggestions for improving storm [10:38am] MootBot: ACTION received: abentley to email ml and gustavo with suggestions for improving storm [10:38am] barry: abentley: for new classes, we really don't want to use the shim [10:39am] abentley: bac: I'll update my branch accordingly. [10:39am] barry: thanks. good discussion. anything else on this or any other non-agenda topic? [10:39am] bac: abentley: thanks. and thanks for bringing the discussion here. it was very useful. [10:40am] abentley: bac: Cool. [10:40am] barry: ok, one last thing before we break [10:40am] barry: [TOPIC] * Mentoring update [10:40am] MootBot: Vote is in progress. Finishing now. [10:40am] MootBot: Final result is 7 for, 0 against. 0 abstained. Total: 7 [10:40am] MootBot: New Topic: * Mentoring update [10:40am] barry: any update from mentors or mentats? [10:42am] barry: no news is good news! [10:42am] barry: #endmeeting [10:42am] MootBot: Meeting finished at 09:42. [10:42am] barry: thanks everyone! [10:42am] gary_poster: bye [10:42am] gary_poster left the chat room. [10:42am] bac: thanks barry [10:43am] abentley: thanks, barry [10:43am] abentley left the chat room. }}}