Code review comment for lp:~klyachin/gomaasapi/101-testserver-extensions

On 2014/04/09 15:19:48, rvb wrote:
> LGTM with a couple of remarks.

> https://codereview.appspot.com/86070043/diff/1/testservice.go
> File testservice.go (right):

https://codereview.appspot.com/86070043/diff/1/testservice.go#newcode482
> testservice.go:482: // AddNodeDetails srores node details, expected in
XML
> format.
> typo: stores
Done.

> https://codereview.appspot.com/86070043/diff/1/testservice_test.go
> File testservice_test.go (right):

https://codereview.appspot.com/86070043/diff/1/testservice_test.go#newcode647
> testservice_test.go:647: expectedResourceURI =

fmt.Sprintf("/api/%s/nodes/mysystemid/macs/aa%%3Abb%%3Acc%%3Add%%3Aee%%3Aff/",
> apiVersion)
> Probably better to express this as the url-escaped version of
> 'aa:bb:cc:dd:ee:ff'. Here and elsewhere.
Done.

https://codereview.appspot.com/86070043/diff/1/testservice_test.go#newcode669
> testservice_test.go:669: c.Assert(err, IsNil)
> This is a bit awkward, can we check the entire set instead of picking
the first
> node and testing that?
The problem is that mac addresses can be in an arbitrary order.
Golang language spec explicitly defines maps as having undefined
ordering of keys.

https://codereview.appspot.com/86070043/diff/1/testservice_test.go#newcode718
> testservice_test.go:718: xmlText := `<?xml version="1.0"
standalone="yes" ?>
> It's a detail but don't we have a equivalent to Python textwrap.dedent
to better
> format this?
Didn't find dedent analogue in Go.
Moved text to a string constant outside of the function.

https://codereview.appspot.com/86070043/