Code review comment for ~pappacena/launchpad:storm_select_related

Revision history for this message
Colin Watson (cjwatson) wrote :

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.

« Back to merge proposal