Merge lp://staging/~allenap/gwacl/get-network-configuration into lp://staging/gwacl

-----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-----

Oh you need a checkPathComponents() call in there too.

Ah, no you don't, there's no user-passed components.

> [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.)

It's not needed because that namespace is not being used anywhere.
Given that things work fine without it, it's just noise.

>
> 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/NetworkConfiguratio
> n"
> xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
> > + <NetworkConfiguration
> xmlns="http://schemas.microsoft.com/ServiceHosting/2011/07/NetworkConfiguratio
> n">

I guess I could leave this in, but I've removed it elsewhere. Again,
this namespace is there so that the document can declare where to find
its schema definition, but (a) it doesn't bother to do so, and (b)
we're not validating anyway. Even if MS do suddenly start declaring
which schema to apply we can safely ignore it.

>
> [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.

Oh yes.

>
> [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

Oops, thanks. Fwiw, the examples are the same (I think).

>
> Everything else looks good, thanks!

Thanks for the review!