Code looks fine, but please fix the tests.
https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go File state/statecmd/resolved_test.go (right):
https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go#newcode18 state/statecmd/resolved_test.go:18: repo *charm.LocalRepository What are conn and repo for?
https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go#newcode21 state/statecmd/resolved_test.go:21: // Ensure our test suite satisfies Suite This is not correct: Suite is a function, and the purpose of the line is to register the suite for running by gocheck.
The following construction (as snarfed from worker/deployer/simple.go):
var _ Context = (*SimpleContext)(nil)
...is the one that ensures that a pointer to a SimpleContext satisfies the Context interface (and does so at compile time).
It seems there are several places this misunderstanding is propagated in this package; please fix those too.
https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go#newcode24 state/statecmd/resolved_test.go:24: func panicWrite(name string, cert, key []byte) error { As far as I can tell, the code from here...
https://codereview.appspot.com/7460047/diff/1/state/statecmd/resolved_test.go#newcode55 state/statecmd/resolved_test.go:55: c.Assert(err, IsNil) ...to here could be replaced with a single call to AddTestingCharm. What am I missing?
https://codereview.appspot.com/7460047/
« Back to merge proposal
Code looks fine, but please fix the tests.
https:/ /codereview. appspot. com/7460047/ diff/1/ state/statecmd/ resolved_ test.go resolved_ test.go (right):
File state/statecmd/
https:/ /codereview. appspot. com/7460047/ diff/1/ state/statecmd/ resolved_ test.go# newcode18 resolved_ test.go: 18: repo *charm. LocalRepository
state/statecmd/
What are conn and repo for?
https:/ /codereview. appspot. com/7460047/ diff/1/ state/statecmd/ resolved_ test.go# newcode21 resolved_ test.go: 21: // Ensure our test suite satisfies
state/statecmd/
Suite
This is not correct: Suite is a function, and the purpose of the line is
to register the suite for running by gocheck.
The following construction (as snarfed from worker/ deployer/ simple. go):
var _ Context = (*SimpleContext )(nil)
...is the one that ensures that a pointer to a SimpleContext satisfies
the Context interface (and does so at compile time).
It seems there are several places this misunderstanding is propagated in
this package; please fix those too.
https:/ /codereview. appspot. com/7460047/ diff/1/ state/statecmd/ resolved_ test.go# newcode24 resolved_ test.go: 24: func panicWrite(name string, cert,
state/statecmd/
key []byte) error {
As far as I can tell, the code from here...
https:/ /codereview. appspot. com/7460047/ diff/1/ state/statecmd/ resolved_ test.go# newcode55 resolved_ test.go: 55: c.Assert(err, IsNil)
state/statecmd/
...to here could be replaced with a single call to AddTestingCharm. What
am I missing?
https:/ /codereview. appspot. com/7460047/