-
Notifications
You must be signed in to change notification settings - Fork 6
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
Refactor mock_step
and deprecate subprocess
kwarg
#25
base: master
Are you sure you want to change the base?
Conversation
# Mock only single step from nested activity to do nothing | ||
true | ||
end | ||
#:mock-subprocess-step end | ||
|
||
#~method | ||
assert_pass new_activity, default_params, {} do |(signal, (ctx, flow_options))| | ||
assert_pass({}, {}, operation: new_activity) do |ctx| |
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.
Why do we need the {}, {}
arguments here, I wonder?
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.
Well, I want to compare ctx[:user]
object here in the 2nd {}
of output here, but I guess it's not possible to compare it's not primitive type!
step Subprocess(subprocess, patch: patch), replace: subprocess, id: subprocess | ||
end | ||
subprocess_path = [subprocess, *subprocess_path] | ||
Trailblazer::Activity::Deprecate.warn caller_locations[0], ":subprocess is deprecated and will be removed in 1.0.0. Pass `subprocess_path: #{subprocess_path}` instead." |
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.
Shouldn't we name it :path
instead of :subprocess_path
(feels clumsy)?
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.
Hmm yeah, but I thought path
resonates more with Activity::Path
and we allow Railway
and FastTrack
too 😄
Having subprocess
in it makes intent clear, how about just subprocess
or subprocesses
?
Refactor
mock_step
method to utilise newDSL::Patch
call and deprecatesubprocess
kwarg. Instead, pass onlysubprocess_path
with full nested path of subprocesses.Old usage
New usage
Passing
subprocess
kwarg will work for now, but it shows deprecation warning and will get removed in next major release.