Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow steps to work without input #68

Merged
merged 1 commit into from
Aug 4, 2017

Conversation

GustavoCaso
Copy link
Member

solves #59

@timriley I decided to give a try to this one. I came up with this solution I wanted to share it so we can discuss it, and see if it's fit with in the library

@GustavoCaso GustavoCaso requested a review from timriley July 19, 2017 06:13
@@ -56,7 +56,12 @@ def call(input)
end

def arity
operation.is_a?(Proc) ? operation.arity : operation.method(:call).arity
case operation
when Proc, Method
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice that the arity function was returning invalid data since we added the functionality of using methods as steps

@@ -4,6 +4,16 @@ module Dry
module Transaction
class StepAdapters
extend Dry::Container::Mixin

module Resolver
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I placed here this code iniside the Resolver module because it was been repeated across all steps. If we do not like this solution I have other options in my mind.

Instead of instantiating the step when registering, we could simply register the class,
later in the call in the step class, we could

def call(input)
   args = [input] + Array(call_args)
   result = step_adapter.new(self, *args).call
   ......
end

This way we could make all the steps inherite from maybe some StepBase class that have this resolve method 😄

@GustavoCaso GustavoCaso changed the title Allow steps towork without input Allow steps to work without input Jul 20, 2017
@timriley timriley merged commit a2d11f9 into master Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants