Add if/unless support#41
Conversation
|
I would suggest rethink that and look again at implementation. It doubles number of lines in builder, adds a lot of complexity makes code harder to follow. Maybe there is a simpler way of implementing this feautre? |
AndriySolonyna
force-pushed
the
SP-8689-conditional-operators
branch
from
3d6e576 to
34e59d1
Compare
petergebala
left a comment
petergebala
left a comment
There was a problem hiding this comment.
Great job @AndriySolonyna Thank you 🙏
| raise ArgumentError, 'Cannot use both :if and :unless on the same step' if opts[:if] && opts[:unless] | ||
|
|
||
| cond = opts[:if] || opts[:unless] | ||
| body = cond.is_a?(Symbol) ? proc { send(cond) } : cond |
There was a problem hiding this comment.
I would rename body what I understand you are ensuring that you get proc/lambda that you can execute later. I am not sure what the best name would be.
- callable
- handler
- callback
- predicate_proc
- condition_proc
| module Opera | ||
| module Operation | ||
| module Builder | ||
| # Parses keyword options passed to a Builder instruction (`step`, |
There was a problem hiding this comment.
imo those comments are not needed. The code is clear
| { predicate: build_predicate(opts) }.compact | ||
| end | ||
|
|
||
| # Translates `:if` / `:unless` (Symbol or Proc) into a single Proc that |
| # to call it in the operation instance scope. | ||
| def condition_met?(instruction) | ||
| operation.instance_exec(&instruction[:predicate]) | ||
| end |
There was a problem hiding this comment.
I also mentioned that on slack. Imo you could inline that above. You don't need 7 lines explaning what is going on here. Devs would know.
| end | ||
| end | ||
|
|
||
| describe 'conditional execution (:if / :unless)' do |
There was a problem hiding this comment.
I would reduce number of test cases. You don't need so many, 430 lines:
probably covering 2 cases with:
step :foo, if: :blabla
and
operation :bar, unless: -> { params[enabled] }
Would be enough. You would cover all cases:
- step
- operation
- if
- unless
- symbol
- proc
And we could reduce number of lines to 40
|
|
||
| ### 0.7.0 - Apr 30, 2026 | ||
|
|
||
| - Add `:if` / `:unless` options to `step`, `operation`, and `operations` for declarative conditional execution. Conditions accept a Symbol (method name) or a Proc/Lambda (evaluated via `instance_exec` in the operation instance scope). Skipped steps do not execute and are not recorded in `result.executions`. For `operation` / `operations`, the conventional `<method>_output` slot in context is set to `nil` when skipped, matching the historical `return Opera::Operation::Result.new` early-exit behavior. Passing both `:if` and `:unless` on the same step raises `ArgumentError` at class load time. |
There was a problem hiding this comment.
we need to update that too. It is for all instructions, not only step/operation
|
|
||
| ### Conditional execution (`:if` / `:unless`) | ||
|
|
||
| `step`, `operation`, and `operations` accept `:if` and `:unless` keyword |
2 participants
Add :if and :unless keyword arguments to step and operation DSL methods. Both accept a symbol (method name) or a lambda/proc.
More details here
https://www.notion.so/profinda/RFC-Conditional-steps-in-opera-349ab9c8a39a802bbf60d1893a472435