search

dry-rb / dry-transaction

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

Replace wisper with dry-events #90

Closed GustavoCaso closed 6 years ago

GustavoCaso commented 6 years ago

This PR replace the gem wisper that we currently use for subscribing functionality with the new dry-events library 🎉 🎉

This change introduces two breaking changes in the library.

  1. The subscriber class has to respond to the methods on_step, on_step_succedd and on_step_failed

  2. The subscriber method will receive an instance of Dry::Events::Event with all the information. This is a breaking change because they will have to access the data with the bracket notation []

I didn't want to make any assumptions about what would be the best arguments for each event, since there is a PR of @timriley https://github.com/dry-rb/dry-transaction/pull/88 that cover that topic.

GustavoCaso commented 6 years ago

Tim regarding the building of full subscriber implementation I think it was because how dry-events gets the method by the subscriber class using the method https://github.com/dry-rb/dry-events/blob/master/lib/dry/events/bus.rb#L55. I guess with that implementation the use of spy could not be possible. Maybe I'm wrong. Unfortunately, I'm not on the computer at the moment

timriley commented 6 years ago

@GustavoCaso Ah right, this makes sense, then :)

GustavoCaso commented 6 years ago

@timriley WDYT? Are you ok merging this one? I have one last concern should we include in this PR making broadcast value consistent?

Including the value in this one.

publish(:step_succeeded, step_name: step_name, args: args)
timriley commented 6 years ago

@GustavoCaso Yeah, I reckon go ahead and put the value: there