On Wed, 10 Apr 2019, Eric Fried wrote:
This suggests a change whereby we move a bunch of module-level methods to instance methods. I actually really like that idea. So for example:
@db_api.placement_context_manager.writer def _set_traits(context, rp, traits): ...
becomes:
class ResourceProvider(object): ... @db_api.placement_context_manager.writer def _set_traits(self, traits): ... # the artist formerly known as 'rp' is now 'self' # and context is self._context
How do folks feel about us making this change?
In the absence of other ideas I'm fine with this because of what I said before: I care most that the code get visited often. My main reservation with this suggestion is that it moves some things in the opposite direction from what I would prefer: I'd like to see (file-based) separation between those methods which are `db_api.placement_context_manager` and those that are not. Or, in other terms: sql generation in other files. This would allow us to have what amounts to three tiers: * web stuff * mumble (might call it controller, but that's not quite right) * sql stuff which I feel provides good contextual cues and affords some opportunities for more robust testing of the sql generation. However, it might be more pain than it is worth as we currently have quite a bit of set() management mixed with sql management. -- Chris Dent ٩◔̯◔۶ https://anticdent.org/ freenode: cdent tw: @anticdent