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

Revision history for this message
Jimmie Butler (jimmiebtlr) wrote :

I'll take a look at the schema/* tests, and see about removing the
extraneous space. Thanks for the feedback.

On Mon, May 5, 2014 at 9:23 PM, John A Meinel <email address hidden>wrote:

> 31 @@ -353,6 +359,7 @@
> 32 vpath[len(vpath)-1] = fmt.Sprint(k.Interface())
> 33 newv, err := c.value.Coerce(rv.MapIndex(k).Interface(), vpath)
> 34 if err != nil {
> 35 +
> 36 return nil, err
> 37 }
> 38 out[newk.(string)] = newv
>
> I think this is a spurious change.
>
> I'm a bit surprised we don't have a direct test of schema/* that would fit
> this change. I believe you have it covered, but only at the charm level.
> Is there a test in schema that you could add that would be a bit more of a
> direct test?
>
> Otherwise, LGTM.
> --
>
> 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

« Back to merge proposal