Introduce the ExecutorBuilder. - #273
Conversation
Sorry, something went wrong.
|
|
||
| // ExecutorBuilder build executor with specific program and args | ||
| type ExecutorBuilder interface { | ||
| NewExecutor(program string, args ...string) Executor |
There was a problem hiding this comment.
or Simply New?
Sorry, something went wrong.
| // ExecutorBuilder build executor with specific program and args | ||
| type ExecutorBuilder interface { | ||
| NewExecutor(program string, args ...string) Executor | ||
| NewShell(cmdline string) Executor |
There was a problem hiding this comment.
or NewExecutorShell ?
Sorry, something went wrong.
| package executor | ||
|
|
||
| type executorBuilderImpl struct { | ||
| } |
There was a problem hiding this comment.
What do you think of keeping the struct private ?
Sorry, something went wrong.
| executor.SetOutputPrefix("---") | ||
| result := executor.Run() | ||
| impl := executor.(*executorImpl) | ||
| impl.outputWriter = buf |
There was a problem hiding this comment.
I was searching for a better way but i cannot think of something
Sorry, something went wrong.
There was a problem hiding this comment.
I think this is fine for now. We could improve this by introducing a executor.SetOutputWriter(writer) at some point.
Sorry, something went wrong.
|
The Executor interface is great. Maybe we should try to ship this first? I don't see the big picture about the builder interface... maybe this should be another PR, also including how we use this builder. |
Sorry, something went wrong.
Why
It would be handy to generate executor from the task context: in the future by example we would be able to keep trace of every requested commands.
How
This PR is introducing a set of interfaces for the ExecutorBuilder but also the Executor itself.
Following PR will add the builder into the TaskContext