-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
[WIP] Invoke commands to combine #82
base: main
Are you sure you want to change the base?
Conversation
This is a useful feature. I already needed it in a couple of places. |
@IvanShamatov This is useful, indeed. What I suggest if it's possible to use message passing, instead of system calls to execute other commands it would be preferrable. But if that is the cost of messing with the command name registry then system calls are fine. Please invest a little bit more time in thinking what happens if the invoked command crashes. |
I think this should just use command objects - is it not possible/feasible now? I do that easily in tests so... |
@solnic I do not pass registry into commands, but to be able to invoke commands from commands expect registry be injected into command object. Actually I'm thinking about that way of implementation and will definitely try it in the nearest time. |
636dfb9
to
40d202a
Compare
40d202a
to
55fd722
Compare
Here is an overview of what got changed by this pull request: Issues
======
- Added 2
See the complete overview on Codacy |
@@ -27,6 +27,7 @@ Gem::Specification.new do |spec| | |||
|
|||
# to update dependencies edit project.yml | |||
spec.add_runtime_dependency "concurrent-ruby", "~> 1.0" | |||
spec.add_runtime_dependency "dry-effects" |
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.
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.
@IvanShamatov as the comment states: to update dependencies you need to edit project.yml
😄
@@ -1,4 +1,5 @@ | |||
# frozen_string_literal: true | |||
require 'dry/effects' |
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.
Issue found: Add an empty line after magic comments.
+1 from me too. Thanks for working on this! |
I'm working on idea of invoking commands from other commands to combine them together.
Say you have views, model, controller generators and you want to make a scaffold command
WDYT ?