Code review comment for lp:~rvb/gwacl/list-endpoints

Revision history for this message
Gavin Panella (allenap) wrote :

[1]

+            endpointsP := vmRole.ConfigurationSets[i].InputEndpoints

I assume the P suffix denotes a pointer. I'm not sure I like that, but
don't change it; I don't have a reason to object other than aesthetics
really.

[2]

+func (suite *suiteListRoleEndpoints) TestWhenGetRoleFails(c *C) {
...
+    c.Check(err, ErrorMatches, "GET request failed [(]404: Not Found[)]")

I wonder if we should revisit these behaviours everywhere in GWACL and
return nil instead of an error. That's easier to use in a conditional,
and a common idiom for absense of something. Errors are a pain to make
decisions about, except nil or not. What do you think? (Utterly out of
scope for this branch; I'm just seeking your opinion.)

review: Approve

« Back to merge proposal