Implement Cmd.Clone() #35
Conversation
cmd.go
Outdated
| c.Args..., | ||
| ) | ||
| clone.Lock() | ||
| defer clone.Unlock() |
There was a problem hiding this comment.
Why lock? clone hasn't been returned yet, so nothing else can be using it.
There was a problem hiding this comment.
Let me try. I remember testing with "-race" in similar cases and the tests were randomly failing because if race conditions.
| // Clone clones a Cmd. All the options are transferred, | ||
| // but the state of the original object is lost. | ||
| // Cmd is one-use only, so if you need to re-start a Cmd, | ||
| // you need to Clone it. |
There was a problem hiding this comment.
Does the cloned Cmd reuse the original's Stdout and Stderr channels?
There was a problem hiding this comment.
Maybe I should document this another way, so it's more clear.
stdout, stderr and the status are dropped when cloning.
Only the public properties of the Cmd are transferred to the clone.
I'll add this ^ to the comment of the function.
There was a problem hiding this comment.
Oh wait, you mean the public Stdout and Stderr?
No, they shouldn't be transferred to the clone, because you want a fresh copy, ready to run from scratch. That's the point of the function.
There was a problem hiding this comment.
Ah yes, right: fresh Stdout and Stderr. 👍
* hfrappier-master: fix: ignore vendor directory. do not source control the same fix: deep framework reverted back to 1.0.1 feat: upgrade deep framework refactor: remove vendor directory . should not be source controlled fix: remove redundant ls fix: list gopath in fix: install goveralls fix: set up modules fix: pull dependencies using go mod feat: remove dep feat: go mod Implement Cmd.Clone() (#35) Chores (#34) Updating typo syntax error Fix Gopkg.lock Test on go1.11, remove go1.8 Test no output (back to 100% coverage) commands can be run in specified folder Fix no output: Buffered=false, Streaming=false Add basic windows support
Implemented simple Clone as discussed in #33