Code review comment for lp:~axwalk/juju-core/wire-up-prechecker-take2
Revision history for this message
| William Reade (fwereade) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
Looking very nice, basically just quibbles. And I'm not quite clear
about the distinction between a nil policy and a PolicyBase{} -- what
exactly governs which you use in a given situation?
https:/
File cmd/jujud/
https:/
cmd/jujud/
state.Policy(nil))
I'm wondering whether it might be good to have a package that implements
environStatePolicy, and includes a state.Open wrapper that always passes
the environ policy in. I'm not sure I can see a use case for alternative
policies at the moment, and if one does show up I think it'll be pretty
easy to add.
As it is I feel it's a bit icky having the nil policies everywhere -- I
think we probably want all our states to work the same in-test and out.
Open to counterarguments though.
https:/
File cmd/jujud/
https:/
cmd/jujud/
I don't really like the nil-return-
sort of NotImplemented error that we handle explicitly in
precheckInstance, please?
https:/
File juju/conn.go (right):
https:/
juju/conn.go:57: policy := state.PolicyBase{}
This bit doesn't have to use the mooted wrapper, ofc.
*but* I don't quite understand what the benefit is of having a null
policy here. What breaks down if we use a real one?
https:/
File state/policy.go (right):
https:/
state/policy.go:17: // for any of the interfaces, but should only return
I think you a word.
https:/
state/policy.go:43: return nil, nil
yeah, `nil, nil` scares me. Please change it :).
https:/