Display executed commands via Verbose Logging #111
Conversation
grayside
changed the title
|
Maybe we should use |
|
The problem there is if we output the log when we create the command, that could be out of sequence when we actually execute it. Certainly wrapping the exec.Command factory is part of the strategy in #97 so I don't object on that point. Would it make sense to extend exec.Cmd into our own struct and shadow Run et al with the verbose logging? |
|
Shadowing might be good, sometimes we don't call |
|
To do this at exec time, I assume we need to shadow each unless the API internals use a common exec call we can shadow. |
…ogging * origin/develop: Fixed bug in libnss-resolver DNS resolution (#116) Added rpm to build, well, rpms Transitioned to different go base to try to head off problems with dynamically linked go binaries tweaking some build flags tweaking CGO messing with ldflags tweaked goreleaser config fixed BaseCommand initialization in delegated commands (#114) Addeed .DS_Store to .gitignore Linux DNS install (#112)
|
Moved command output to yellow for better visibility. Shifted to more of an object wrapper model. Stopped implementation for approach review. |
util/shell_exec.go
Outdated
|
|
||
| // Run runs a command via exec.Run() without modification or output of the underlying command. | ||
| func (x Executor) Run() error { | ||
| return x.cmd.Run() |
There was a problem hiding this comment.
Do we want to log all commands that are run? Or only commands that force/stream?
There was a problem hiding this comment.
Probably all commands, but I wanted to lay out the initial API for it more flexibly so the refactoring can go either way.
|
I like how this is looking a lot better. The code that needs to run commands look really no different and is still pretty easy to read, and the Executor looks pretty simple. |
util/shell_exec.go
Outdated
| // StreamCommand sets up the output streams (and colors) to stream command output if verbose is configured | ||
| func StreamCommand(cmd *exec.Cmd) error { | ||
| return RunCommand(cmd, false) | ||
| return Executor{cmd}.Execute(false) |
There was a problem hiding this comment.
You know, we could make this and ForceStreamCommand even easier to read on the caller side if we just take the command and args here instead of the exec.Cmd and call exec.Command just like the Executor.Execute
|
Too much?
|
|
Note that in the example above, we see that our use of machine.GetData() could maybe do some static caching. Likely a nice little performance optimization. |
grayside
changed the title
* origin/develop: Display executed commands via Verbose Logging (#111) Better cross platform builds (#118) Fixing Outrigger Dashboard on Linux (#117) Fixed bug in libnss-resolver DNS resolution (#116) Added rpm to build, well, rpms Transitioned to different go base to try to head off problems with dynamically linked go binaries tweaking some build flags tweaking CGO messing with ldflags tweaked goreleaser config
Finally getting back to the learnification.
This is still in-progress but wanted to get an approach confirmation before continuing.
Fixes #5