Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Diff of everything #1
Conversation
natefinch
added some commits
Jun 17, 2015
davecheney
commented on deputy.go in bb01ab2
Jun 17, 2015
|
Please delete this. |
davecheney
commented on deputy.go in bb01ab2
Jun 17, 2015
|
I think this should be a function, not a method func Run(cmd *exec.Cmd, opts ...Option) { ... } |
natefinch
added some commits
Jun 17, 2015
ericsnowcurrently
commented
Jun 19, 2015
|
LGTM |
natefinch
added some commits
Jun 22, 2015
rogpeppe
reviewed
Jun 23, 2015
| + } | ||
| + | ||
| + errsrc := &bytes.Buffer{} | ||
| + if d.Errors == FromStderr { |
rogpeppe
Jun 23, 2015
How about defining a multiWriter function:
func dualWriter(w0, w1 io.Writer) io.Writer {
if w1 == nil {
return w0
}
return io.MultiWriter(w0, w1)
}
then:
switch d.Errors {
case FromStderr:
cmd.Stderr = dualWriter(errsrc, cmd.Stderr)
case FromStdin:
cmd.Stdout = dualWriter(errsrc, cmd.Stdout)
}
?
rogpeppe
reviewed
Jun 23, 2015
| + } | ||
| + | ||
| + if err != nil && errsrc.Len() > 0 { | ||
| + return fmt.Errorf("%s: %s", err, strings.TrimSpace(errsrc.String())) |
rogpeppe
reviewed
Jun 23, 2015
| + // Errors describes how errors should be handled. | ||
| + Errors ErrorHandling | ||
| + // StdoutLog takes a function that will receive lines written to stdout from | ||
| + // the command. |
rogpeppe
Jun 23, 2015
// Note that this will override the Stdout field if it's already set.
(and likewise for StderrLog below)
?
rogpeppe
reviewed
Jun 23, 2015
| + } | ||
| + | ||
| + err := d.run(cmd) | ||
| + |
rogpeppe
reviewed
Jun 23, 2015
| +func pipe(log func(string), r io.Reader, errs chan<- error) { | ||
| + scanner := bufio.NewScanner(r) | ||
| + for scanner.Scan() { | ||
| + w := scanner.Text() |
rogpeppe
reviewed
Jun 23, 2015
| + | ||
| +// Run starts the specified command and waits for it to complete. Its behavior | ||
| +// conforms to the Options passed to it at construction time. | ||
| +func (d Deputy) Run(cmd *exec.Cmd) error { |
rogpeppe
Jun 23, 2015
// Note that, like cmd.Run, Deputy.Run should not be used with
// StdoutPipe or StderrPipe.
rogpeppe
reviewed
Jun 23, 2015
| + } | ||
| + if d.StdoutLog != nil { | ||
| + var err error | ||
| + d.stdoutPipe, err = cmd.StdoutPipe() |
rogpeppe
Jun 23, 2015
If both StdoutLog and StderrLog are set, I wonder if it would be
better to just use a single pipe - that way the stdout and stderr
log messages cannot be reordered.
rogpeppe
reviewed
Jun 23, 2015
| + | ||
| +func (d Deputy) wait(cmd *exec.Cmd, errs <-chan error) error { | ||
| + var err1, err2 error | ||
| + if d.stdoutPipe != nil { |
rogpeppe
Jun 23, 2015
// Note that it's important that we wait for the pipes
// to be closed before calling cmd.Wait otherwise
// Wait can close the pipes before we have read
// all their data.
?
rogpeppe
reviewed
Jun 23, 2015
| + StdoutLog func(string) | ||
| + // StdoutLog takes a function that will receive lines written to stderr from | ||
| + // the command. | ||
| + StderrLog func(string) |
rogpeppe
Jun 23, 2015
I wonder if it would be better for these to take a []byte argument to save an allocation,
which can be reasonably significant for verbose logging.
rogpeppe
commented
Jun 23, 2015
|
LGTM with some comments and suggestions. |
natefinch
added some commits
Jun 26, 2015
natefinch commentedJun 18, 2015
No description provided.