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

Revision history for this message
Raphaƫl Badin (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

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.

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?

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?

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

« Back to merge proposal