Stylistic issues

[bkeroackdsc]

On first glance, this library seems a bit "method happy" so to speak. For example, the usage of aws.String(foo) instead of the more idiomatic (and simpler/faster) &foo. There seems to be a similar aesthetic with the Credentials subpackage -- why can't I for example pass the access id/secret key directly to s3.New()?

Some code review should be done to make this library more Go-community friendly before freezing the API.

[lsegal]

@bkeroackdsc there are a few discussions about style going on in other issues, I would recommend looking through open issues and jumping in with specifics there, that said,

For example, the usage of aws.String(foo) instead of the more idiomatic (and simpler/faster) &foo.

Nothing stops you from using &foo when possible, and in fact, if you are actually have a variable foo, you should use &foo. The SDK does not make any recommendations in any of our documentation to use aws.String() on existing string variable values. The only place our documentation uses aws.String() and the like are for string (and number) literals, and this is due to a limitation in Go, namely, you cannot take the address of a literal without some variable assignment. For example, you cannot do:

input := &s3.CreateBucketInput{Bucket: &"bucket-name"}

And it would be awkward to have to do:

bucket := "bucket-name"
input := &s3.CreateBucketInput{Bucket: &bucket}

The aws.String() helper allows this to be written in one line:

input := &s3.CreateBucketInput{Bucket: aws.String("bucket-name")}

Hopefully that explains the logic there.

There seems to be a similar aesthetic with the Credentials subpackage -- why can't I for example pass the access id/secret key directly to s3.New()?

This was an intentional change to discourage the use of hardcoded credentials passed to constructors. I would suggest reading through the Getting Started Guide, specifically Configuring Credentials. It is not recommended to hardcode credentials and there is usually a better way to configure credentials in the SDK-- in fact, the SDK does pretty good automatic detection of credentials that, in most cases, should make it so you don't need to pass credentials into your constructor at all. This is the recommended way to initialize S3, for instance:

svc := s3.New(nil) // no credentials needed

[bkeroackdsc]

Thanks for explaining some of the rationale behind the decisions. It brings up another point about the pervasive use of pointers in this library--any insight into why that was done? It makes using this library pretty cumbersome.

[lsegal]

@bkeroackdsc you can find the discussion about pointers in a few of our existing discussion issues as mentioned above; #114, for example. Instead of rehashing the explanation, I would suggest reading there. The short explanation is that most parameters (if not all) can be omitted, and it is not easily (or feasible) to differentiate Go's zero values from omitted values. You can see the linked issue for more details.

[porjo]

Some code review should be done to make this library more Go-community friendly before freezing the API.

+1 to this. I've just had a quick look over the API for the first time and it feels bloated. Just to take one example cloudformation.CreateStackInput vs cloudformation.UpdateStackInput are virtually identical. Is that duplication really necessary?

[lsegal]

@porjo

Is that duplication really necessary?

The methods exposed mimic the operations of the underlying surface. The low-level SDK operations are designed to be a 1:1 mapping of the API operations in the exposed services. The benefit of this design is that you can always trace the parameters to the underlying API documentation. From a documentation perspective, you'll note that the docstrings are unique and occasionally provide fairly important details about contextual details in creating and updating stacks. This information would not be easily captured if these two structures were shared.

[jasdel]

Thank you very much for your feedback. The design of pointers, style, and calling method is complex and very important to the long term maintainability of the SDK. We have a few issues which are duplicating each other and I'd like to collapse the discussion down to a single issue. Lets use #363 for further discussion.

Duplicates: #363
Related: #265, #284, #294