Code review comment for lp:~thumper/juju-core/user-display-name

LGTM - only minor comments and praise added in-line. The user add tests
look much better.

One thing: can this actually land without schema migration support?

https://codereview.appspot.com/102970043/diff/1/cmd/juju/user_add.go
File cmd/juju/user_add.go (left):

https://codereview.appspot.com/102970043/diff/1/cmd/juju/user_add.go#oldcode44
cmd/juju/user_add.go:44: Args: "<username> <password>",
Sorry for missing this when I implemented --password

https://codereview.appspot.com/102970043/diff/1/cmd/juju/user_add.go
File cmd/juju/user_add.go (right):

https://codereview.appspot.com/102970043/diff/1/cmd/juju/user_add.go#newcode109
cmd/juju/user_add.go:109: return err
errors.Trace() here too?

https://codereview.appspot.com/102970043/diff/1/state/api/usermanager/client.go
File state/api/usermanager/client.go (left):

https://codereview.appspot.com/102970043/diff/1/state/api/usermanager/client.go#oldcode37
state/api/usermanager/client.go:37: p := params.EntityPasswords{Changes:
[]params.EntityPassword{u}}
These names were terrible! Glad you've fixed it.

https://codereview.appspot.com/102970043/diff/1/state/api/usermanager/client.go
File state/api/usermanager/client.go (right):

https://codereview.appspot.com/102970043/diff/1/state/api/usermanager/client.go#newcode37
state/api/usermanager/client.go:37: Changes:
[]params.ModifyUser{{Username: username, DisplayName: displayName,
Password: password}},
This is much more readable.

https://codereview.appspot.com/102970043/diff/1/state/api/usermanager/client_test.go
File state/api/usermanager/client_test.go (right):

https://codereview.appspot.com/102970043/diff/1/state/api/usermanager/client_test.go#newcode42
state/api/usermanager/client_test.go:42: err :=
s.APIState.Call("UserManager", "", "AddUser", userArgs, results)
Maybe add a comment here about why s.APIState.Call("UserManager",...) is
used here instead of s.usermanager.AddUser()? I understand why but had
to think about it for a bit.

https://codereview.appspot.com/102970043/diff/1/state/apiserver/charms_test.go
File state/apiserver/charms_test.go (left):

https://codereview.appspot.com/102970043/diff/1/state/apiserver/charms_test.go#oldcode39
state/apiserver/charms_test.go:39: password, err :=
utils.RandomPassword()
Play devil's advocate: this change means the user has a fixed password
instead of a random one. Are you sure that doesn't negatively impact any
existing tests? I can't think of why it would but it is a test
functionality change.

https://codereview.appspot.com/102970043/diff/1/state/user_test.go
File state/user_test.go (right):

https://codereview.appspot.com/102970043/diff/1/state/user_test.go#newcode45
state/user_test.go:45: func (s *UserSuite) makeUser(c *gc.C) *state.User
{
addSomeUser might be a better name, indicating that a test/throwaway
user is being created. It also hints at the username being used.

https://codereview.appspot.com/102970043/