← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] ~pappacena/launchpad:storm_select_related into launchpad:master

 

I think I see where you're going with this, and in principle it seems like a reasonable thing to add.  This is almost a worst case for a third-party extension to Storm though; you're having to contort things a bit to cope with being in a subclass, and I'd find it a *lot* easier to review as an MP against lp:storm itself.  (The people more or less active in maintaining Storm at the moment seem to be me and Simon Poirier.)

I'd also suggest talking with Free Ekanayaka, who's still around (though not working on Storm much) and you should be able to get hold of him.  It was a long time ago and I'm not quite sure where it went, but https://bazaar.launchpad.net/~free.ekanayaka/storm/eager-loading/revision/361 seems like it's trying to do some of the same things that you're trying to do, and to my mind looks rather cleaner and better-integrated with Storm's reference mechanisms; so it would be worth asking him whether there were any fundamental obstacles there.  I haven't reviewed either approach in a lot of fine detail, though, and both would need lots of testing.
-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/377968
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:storm_select_related into launchpad:master.


References