Code review comment for lp:~jimmiebtlr/juju-core/fix-no-peers-charm

Ok, misunderstood that little bit. Basically it should return an empty map
rather than nil even if peers, requires, and provides are not mentioned in
the charm file. That will require a change in a different location,
meaning we may not need to change the map behavior at all.

On Tue, May 20, 2014 at 12:48 AM, William Reade <<email address hidden>
> wrote:

> LGTM, I'd accept this as-is, but please take a little look at the
> nil/map{} issue. If it's too complex, let me know, and I'll approve it.
>
> Dave, thanks for the caution; but schema isn't as heavily used as one
> might think (especially the map bits), and I think this is a good
> change.
>
>
> https://codereview.appspot.com/98410043/diff/1/charm/meta_test.go
> File charm/meta_test.go (left):
>
>
> https://codereview.appspot.com/98410043/diff/1/charm/meta_test.go#oldcode163
> charm/meta_test.go:163: c.Assert(meta.Peers, gc.IsNil)
> It's this bit that bugs me, that I alluded to yesterday -- having a Meta
> type that sometimes has nil and sometimes has {} feels weirdly
> inconsistent. It's not really a very big deal, but I'd appreciate a fix
> if it's not excessively complex to change.
>
> https://codereview.appspot.com/98410043/diff/1/schema/schema.go
> File schema/schema.go (right):
>
> https://codereview.appspot.com/98410043/diff/1/schema/schema.go#newcode297
> schema/schema.go:297: // handles case where map is empty
> treat nil value as empty map :)
>
> https://codereview.appspot.com/98410043/
>
> --
>
> https://code.launchpad.net/~jimmiebtlr/juju-core/fix-no-peers-charm/+merge/218195
> You are the owner of lp:~jimmiebtlr/juju-core/fix-no-peers-charm.
>

--
Thanks,
Jimmie Butler