-
Notifications
You must be signed in to change notification settings - Fork 8
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
Implement If() macro #49
base: master
Are you sure you want to change the base?
Conversation
This commit adds an `If()` macro to conditionally execute steps in an operation. It's implementation is based on the implementation of the `Wrap()` macro to execute the nested sub-activity. Co-authored-by: Roland Schwarzer <[email protected]>
Just noticed there is another PR for an If macro already (#43). From my point of view our implementation feels a bit more like trailblazer and has some better test coverage. |
Should we allow two signatures here, one without |
Yeah, I think we should! BTW, does |
Okay @richardboehme and I agreed that introducing a new signature without |
After pondering over this PR for ... half a year, I'm wondering, couldn't we reuse
|
@apotonick We tried to reuse |
class If < Macro::Strategy | ||
def self.call((ctx, flow_options), **circuit_options) | ||
name = @state.get(:name) | ||
ctx[:"result.condition.#{name}"] = flow_options[:decision] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is ctx[:"result.condition.#{name}"]
important to keep?
This commit adds an
If()
macro to conditionally execute steps in an operation. It's implementation is based on the implementation of theWrap()
macro to execute the nested sub-activity.Right know if one needed a step to execute only if a specific condition is true we used early returns:
With this macro we can transform that into:
If you have any ideas on how to further improve this, let us know.