state/api/params: consistent CharmURL

state/api/params: consistent CharmURL

Also add comments as suggested by dfc.

https://code.launchpad.net/~rogpeppe/juju-core/257-api-consistent-charmurl/+merge/155316

(do not edit description out of merge proposal)

[rog, 2013-03-25 18:34:32]

Please take a look.

[rog, 2013-03-25 18:35:55]

Please take a look.

[dimitern, 2013-03-25 18:59:22]

Nicer, LGTM.

[dave_cheney.net, 2013-03-25 23:05:06]

This looks nicer. I'm still unsure about the comment.

https://codereview.appspot.com/7931045/diff/5001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/7931045/diff/5001/state/api/params/params.go#n...
state/api/params/params.go:245: // must unmarshal correctly into these
documents.
This sounds important, but don't all our documents have to round trip perfectly
?

I apologize for the pedantry, but this comment does not explain why the code
that follows is important, nor how to care for it.

[rog, 2013-03-26 08:20:44]

Please take a look.

https://codereview.appspot.com/7931045/diff/5001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/7931045/diff/5001/state/api/params/params.go#n...
state/api/params/params.go:245: // must unmarshal correctly into these
documents.
On 2013/03/25 23:05:06, dfc wrote:
> This sounds important, but don't all our documents have to round trip
perfectly
> ?

No, these ones do not. They are only used for reading from mongo, not for
writing to it. They do have to round-trip through JSON, but that's true of all
the structs in this package.

> I apologize for the pedantry, but this comment does not explain why the code
> that follows is important, nor how to care for it.

I've made some more changes, but I'm not sure that it's what you want. Perhaps
you could suggest something?

[TheMue, 2013-03-26 08:46:27]

Overall LGTM, but I have to look more at the concept of having the internal
documents exactly mirrored.

[fwereade, 2013-03-26 09:04:56]

On 2013/03/26 08:46:27, TheMue wrote:
> Overall LGTM, but I have to look more at the concept of having the internal
> documents exactly mirrored.

It's not nice, but it's expedient :).

[rog, 2013-03-27 11:58:59]

In a private message, Dave Cheney says "go ahead" which I take as an implicit
LGTM, so I'll submit this now.

[rog, 2013-03-27 12:07:36]

*** Submitted:

state/api/params: consistent CharmURL

Also add comments as suggested by dfc.

R=dimitern, dfc, TheMue, fwereade
CC=
https://codereview.appspot.com/7931045