Code review comment for lp://staging/~themue/golxc/001-added-sudo-and-first-test

Revision history for this message
William Reade (fwereade) wrote :

Not sure about the tests -- it looks like there's plenty of opportunity
from pollution from failures. Is there some motivation for this
behaviour that I've missed?

Also, it kinda looks as though the tests will fail if your system is
already running a container. Can you see any way around this?

https://codereview.appspot.com/6852065/diff/7007/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/7007/golxc.go#newcode217
golxc.go:217: _, _, err := run("lxc-wait", "-n", c.name, "-s",
combinedStates)
I guess LXC will complain about impossible state combinations?

https://codereview.appspot.com/6852065/diff/7007/golxc.go#newcode257
golxc.go:257: // String returns a short information about the container.
s/a short/brief/

perhaps?

https://codereview.appspot.com/6852065/diff/7007/golxc_test.go
File golxc_test.go (right):

https://codereview.appspot.com/6852065/diff/7007/golxc_test.go#newcode20
golxc_test.go:20: lc.Destroy()
Shouldn't these be destroyed inline via defer?

https://codereview.appspot.com/6852065/

« Back to merge proposal