dry-rb / dry-transaction

Business transaction DSL
http://dry-rb.org/gems/dry-transaction
MIT License
468 stars 55 forks source link

Allow replacement of internal steps #109

Closed timriley closed 6 years ago

timriley commented 6 years ago

So @GustavoCaso and I have been talking about addressing #75 and making it possible to provide replacement for "internal" transaction steps (i.e. steps which are backed by instance methods only). The inability to do this feels to me the only big gap we left after moving to class-based transactions, and given an increased in the number of people using container-less transactions, it would be good to get it in place.

@GustavoCaso, I've taken a different approach to what you've done recently, and I think it might work out.

This PR makes one big change in the way we understand steps:

Then, when an "internal" step ("internal" meaning a step implanted by an instance method only) has a matching object passed to the initializer, that object fully replaces whatever was previously provided by the instance method

This is a breaking change:

So I don't think the breaking changes here are too big, and I think the result is actually a much clearer separation of behaviours.

Plus, I'm much happier to merge something like this in given how small a change it ended up being.

What do you think?

@flash-gordon would be great to get your input, too.

flash-gordon commented 6 years ago

this looks fine to me btw, probably someone will come up with a better idea in future but nothing comes into my mind atm

jcmfernandes commented 6 years ago

Stopping by to say thanks, as I hit this problem earlier today, and while debugging I ended up realizing that it was fixed in the latest release :smiley: thanks for the great work everyone!