Code review comment for lp:~allenap/gwacl/destroy-deployment

On Wednesday 03 Jul 2013 09:53:28 you wrote:
> This would be very unweildy because you have to initialise like so:
>
> request := &ShutdownRoleRequest{
> DeploymentRequest: DeploymentRequest{
> ServiceName: "...",
> DeploymentName: "...",
> },
> RoleName: "...",
> }
>

Oooo wtf? Really? I thought embedding just saves the typing!

> >From what I've learned so far, struct embedding is just a bit of
>
> misleading sugar, but probably there are some use cases out there
> somewhere.
>
> Where they are the same, we can do an alias-like thing:
>
> type GetDeploymentRequest DeploymentRequest

That works equally well for now, but makes it a little harder to add more
params later.

Having said all that I'm now tending towards just duplicating stuff.

>
> Let's do a sweep at the end for this stuff, and live with a small
> amount of duplication for now.

Yeah, agreed.

>
> > I rebelled against you the other day and used backslashes instead of
> > the []s - I think they obfuscate the string too much.
> >
> > c.Check(err, ErrorMatches, `GET request failed \(404: Not Found\)`)
> >
> > is clearer IMO.
> :
> :) We shall have to agree to differ. My in-brain regex parser has
>
> evolved different precedence rules.

See, there's your problem, your brain can parse regex!

> Thanks for the review!

A pleasure.