Code review comment for lp://staging/~allenap/gwacl/get-network-configuration

Revision history for this message
Julian Edwards (julian-edwards) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

review: needs-information

Needs info because of [1]

[1]

> -type SetNetworkConfiguration struct { - XMLName
> xml.Name `xml:"NetworkConfiguration"` +type
> NetworkConfiguration struct { XMLNS string
> `xml:"xmlns,attr"` - XMLNS_XSI string
> `xml:"xmlns:xsi,attr"`

Why remove this? It is required for requests. (It's also in the
response from GetNetworkConfiguration but is ignorable there I guess.)

You also changed it in the test template (which is copied directly
from the msdn web page):

> - <NetworkConfiguration
xmlns="http://schemas.microsoft.com/ServiceHosting/2011/07/NetworkConfiguration"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
> + <NetworkConfiguration
xmlns="http://schemas.microsoft.com/ServiceHosting/2011/07/NetworkConfiguration">

[2]

> -func (s *SetNetworkConfiguration) Serialize() (string, error) {
> +func (s *NetworkConfiguration) Serialize() (string, error) {
> return toxml(s) }
>
> +func (s *NetworkConfiguration) Deserialize(data []byte) error { +
> return xml.Unmarshal(data, s) +} +

A small point, but "s" is no longer appropriate.

[3]

> +func (suite *xmlSuite) TestNetworkConfigurationDeserialize(c *C)
> { + // Template from + //
> http://msdn.microsoft.com/en-us/library/windowsazure/jj157181.aspx

You quoted the URL for "set config". I think this should be the "get
config" at:
http://msdn.microsoft.com/en-us/library/windowsazure/jj157196.aspx

Everything else looks good, thanks!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlHcqY0ACgkQWhGlTF8G/Hcm7gCeIJUSZ1jQYX9dnpvErUAEechu
rZEAnjhnxnaeqz0seeYphG1osAwEvn9e
=sfAh
-----END PGP SIGNATURE-----

« Back to merge proposal