search

adomokos / light-service

Series of Actions with an emphasis on simplicity.
MIT License
837 stars 67 forks source link

Organizers call each action with the same context #149

Closed thegene closed 6 years ago

thegene commented 6 years ago

Rather than passing the return of each action call to the subsequent action, organizers now call each action with the same context, allowing actions to mutate the same context action.

Am mainly opening this up for discussion as this feels like a pretty fundamentally breaking change.

The issue this solves for is when an organizer is within another organizer as an action the action proceeding this organizer will be called with the return from call rather than the context object. This will of course then fail because the action is being called with something other than a LightService::Context.

adomokos commented 6 years ago

Interesting change. I've just ran your proposed change on our code base and it passed all the tests. Let's chat about it and figure out if we want this to be merged or not.

Thanks for submitting it!

adomokos commented 6 years ago

@thegene - I'd like to final review and merge this PR. Can you please make Rubocop happy?

adomokos commented 6 years ago

We might need to add a bit better specs to this change.

I grabbed your changes and reset the WithReducer class to be the same as we have it in master right now. I switched back to reduce from each_with_object. After I polished the specs you added, all your new specs were passing with the original code. See my code tweak here.

The ReturningOrganizer just sets the :foo value in the context, but it was not returning the context. Now I am realizing, that by not returning the context there is the benefit of using each_with_object, which is the whole point of this PR. 👍 💯 I have never used Organizers without Actions, I guess that's why I was a bit confused by it.

I'd suggest renaming that organizer to be NotExplicitlyReturningContextOrganizer, to be more clear. What do you think?

adomokos commented 6 years ago

Thank you!!! :100: