Possibly build on top of ActiveSupport::Callbacks?
Closed this issue · 9 comments
I noticed when reading the ActiveSupport code that ActiveSupport::Callbacks seem to implement a similar design that modware currently does. Would it be a good idea to build on top of that framework instead?
I'm not sure about the precise details (of either library), but it was just from a cursory glance at the code.
I noticed when reading the ActiveSupport code that ActiveSupport::Callbacks seem to implement a similar design that modware currently does. Would it be a good idea to build on top of that framework instead?
Yes, probably so. When I was going through a few rounds of designing/refactoring schema_monkey & schema_plus_core I originally thought that ActiveSupport::Callbacks wouldn't quite fit, so I used a different middleware gem... but that one turned out not to be a great fit so I rolled my own; but I may have indeed gone full circle.
Using ActiveSupport would probably make the whole setup more appealing to rails devs too.
I haven't looked at it in detail, but I imagine modware's API for defining the callbacks isn't quite the same as ActiveSupport::Callbacks. So schema_monkey would probably need to put a thin layer around it to maintain the same behavior. (Or maybe better, just change modware to use ActiveRecord::Callbacks internally.)
Then another question would be whether to switch schema_monkey use to ActiveRecord::Callbacks' API directly (with the followup question of whether to support both modware API and ActiveRecord API, and deal with that, or only ActiveRecord and then deal with the breaking changes to existing clients)
Yes, probably so. When I was going through a few rounds of designing/refactoring schema_monkey & schema_plus_core I originally thought that ActiveSupport::Callbacks wouldn't quite fit, so I used a different middleware gem... but that one turned out not to be a great fit so I rolled my own; but I may have indeed gone full circle.
Such is the life of a software engineer :)
I haven't looked at it in detail, but I imagine modware's API for defining the callbacks isn't quite the same as ActiveSupport::Callbacks. So schema_monkey would probably need to put a thin layer around it to maintain the same behavior. (Or maybe better, just change modware to use ActiveRecord::Callbacks internally.)
Maybe what can be tried is to change modware to use ActiveSupport::Callbacks internally. Then we'll see how much adaptation modware needs to fit AS::Callbacks. If it really is thin, we can just scrap it altogether and directly use AS::Callbacks, then the second part of the question is answered.
If modware isn't a 1-to-1 wrapper around AS::Callbacks, then it should preserve behaviour. I don't foresee the behaviour to be that different, however.
I think it would be preferable to use AS::Callbacks API and behaviour where possible, unless there's a really good reason to act otherwise. This would allow the greater Rails community to directly contribute.
That all sounds good.... you want to dive in and try it?
I suspect that the wrapper will be thin, but that scrapping it entirely will mean needing to change how the client modules define their callbacks. Still it may be worth it.
I'm a little short on time at the moment, but I'll see what can be done about it when time permits.
I'm a little short on time at the moment
Likewise, except that I'm a lot short on time. Today I did aside a chunk of time (I'm on a long airplane flight right now) trying to see if I can spend it finally spinning the last two gems out of schema_plus and "officially" release 2.0.0. Then I'll have to turn my attention to other things that have been neglected. I'll do critical bug fixes and happy to kibbitz on yours and other people's coding, but will have to take a break from hard core hacking on it myself.
but I'll see what can be done about it when time permits.
cool, whenever you can will be great. (BTW also at some point I should just give you collaborator access to the SchemaPlus organization so you can check things in yourself...)
Looks like you made progress today 😛
I finally dug into this -- it doesn't seem possible because modware passes arguments down. AS::Callbacks only supports modifying the current object, so they have slightly different use cases. Closing.
Ah well, thanks for looking into it. And thanks for tidying up the issue list :)