Code review comment for lp:~themue/golxc/003-added-network

Sorry to keep on about these things :(. I'm ok to say LGTM in the
interest of progress, assuming the points raised below are addressed or
refuted :). Chat to me if there's doubt.

https://codereview.appspot.com/6849102/diff/11001/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6849102/diff/11001/golxc.go#newcode314
golxc.go:314: // analyzing it in runError() in case of an error.
// LXC tools do not use stdout and stderr in a predictable
// way; based on experimentation, the most convenient
// solution is to combine them and leave the client to
// determine sanity as best it can.

https://codereview.appspot.com/6849102/diff/11001/golxc.go#newcode318
golxc.go:318: }
On 2013/02/05 14:21:32, TheMue wrote:
> On 2013/02/04 14:51:30, fwereade wrote:
> > And if there was no error, but extra stuff was printed to stderr,
we'd just
> hand
> > it back below... so the comment doesn't quite address my concern,
which is
> about
> > getting unwanted output but no error.

> That's exactly the problem. Some of the lxc commands simply use stdout
for both
> types, positive and error messages, others use stderr. So far I've
never seen
> mixed output and also signalling an error seems to work correctly.

> How would you write this as comment? Simply longer and more explicit?

Suggested above?

https://codereview.appspot.com/6849102/diff/11001/golxc.go#newcode326
golxc.go:326: if strings.HasPrefix(l, name+": ") {
I'm not sure what we get out of hiding parts of the output in the case
of error. Comment perhaps?

https://codereview.appspot.com/6849102/diff/18001/golxc_test.go
File golxc_test.go (right):

https://codereview.appspot.com/6849102/diff/18001/golxc_test.go#newcode173
golxc_test.go:173: // nonExistingPath returns a non-existing file path.
I'm not sure how this solution is superior to a
`filepath.Join(c.MkDir(), "foo")` at the call site.

https://codereview.appspot.com/6849102/