launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Thanks for the review.

> It's incredibly tempted to optimize away unused error return values, isn't it?
> MAASObject.URL(), for instance: calling it would be easier if it didn't have
> that double return which it currently does not need.  But once you do that,
> there's no compatible way back to allowing a function to fail.  I wonder how
> Go's standard library will hold up — adding error returns everywhere would
> make it all but unusable, but without them, the only long-term solution is to
> accept panic/recover as conventional exception handling.

I was thinking exactly along the same lines.  I think we could ensure that a maasobject is a map and contains the resource_uri key and then URI() and URL() could simple panic if there is any problem fetching the resource_uri value (as this should never happen).

> 
> In util.go, AppendSlash() is a bit of a misnomer.  Maybe
> EnsureTrailingSlash()?

Good point, done.

> For implementing it, try strings.HasSuffix("/").

Oh, didn't know about that one, done.

> In util_test.go, TestAppendSlashDoesNotAppendsIfPresent() has an "Appends"
> that should read "Append".

Well spotted, fixed.
-- 
https://code.launchpad.net/~rvb/maas/gomaasapi-baseURL/+merge/145180
Your team MAAS Maintainers is subscribed to branch lp:~maas-maintainers/maas/gomaasapi.