← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~rvb/maas/gomaasapi-baseURL into lp:~maas-maintainers/maas/gomaasapi

 

Review: Approve

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.

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

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

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


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


Follow ups

References