Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

naming conventions: data types (Props & Options) #1740

Closed
rix0rrr opened this issue Feb 12, 2019 · 4 comments
Closed

naming conventions: data types (Props & Options) #1740

rix0rrr opened this issue Feb 12, 2019 · 4 comments
Assignees
Labels
@aws-cdk/core Related to core CDK functionality package/awscl Cross-cutting issues related to the AWS Construct Library

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 12, 2019

We need to have a naming convention around data objects, used as (optional) keyword arguments.

They are used for three purposes:

  • To configure a Construct, via its constructor (example: new Bucket(scope, id, PROPS)).
  • As options to a function or method, that customize its behavior (example: eventRule.addTarget(target, TEMPLATE))
  • To configure a Construct indirectly, via a method on an existing construct (example: loadBalancer.addListener(id, LISTENERPROPS))

In the TypeScript/JavaScript world, data objects used for configuration are usually called options or opts.

In the CloudFormation world, input data for configuring resources are called properties.

Because of our duality (mapping between programming and CloudFormation), we are split between calling these data objects properties and options.

  • For the arguments to a constructor, we've most definitely settled on props, e.g. BucketProps.
  • For the arguments to a method, we've varying between props and options.
  • Same as for methods that construct a new construct.

I propose that we adopt the rule: Props for construct, Options for methods.

So in the examples above, the type names for the configuration objects would be:

  • BucketProps, no change.
  • AddTargetOptions.
  • AddListenerOptions.

Thoughts?

@eladb
Copy link
Contributor

eladb commented Feb 12, 2019

A few notes:

  • The naming convention we need is not for data types (in my mind) but more for keyword argument types. In the CDK this is almost 1:1, but just to make sure we are focused on the right semantics. To me, the missing convention is how do we call "keyword argument types".
  • Another convention that will be useful to define around this is how to call "common properties". It is quite common that we need to split properties into two in order to reuse across either multiple sub-types or to allow some options to be passed in a newXxx method (but not all). API Gateway has many examples.

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Feb 12, 2019

The naming convention we need is not for data types (in my mind) but more for keyword argument types.

Don't really agree to disregard the type name. The argument name is positional, so doesn't really impact the code you're writing (yes, it will affect the docs and the code insight, but not the code as written). In languages like Java, the type name will be front and center as well. I do agree we need to standardize both.

Another convention that will be useful to define around this is how to call "common properties".

Agreed, and it's been floating through my mind as well. Kind of a crossover topic between this ticket and #1741 as well. I would like to (sardonically :P) point out that that discussion is about the type name as well, not about the argument name.

@eladb
Copy link
Contributor

eladb commented Feb 12, 2019

I wasn't implying to disregard the type name, just that the scope of this convention should be "what should be the type name for types that are used for keyword arguments"

@eladb eladb added the @aws-cdk/core Related to core CDK functionality label Feb 28, 2019
@eladb
Copy link
Contributor

eladb commented Mar 11, 2019

I am leaning towards just calling everything "Options": microsoft/TypeScript#23552

@eladb eladb self-assigned this Aug 12, 2019
@eladb eladb added the package/awscl Cross-cutting issues related to the AWS Construct Library label Aug 28, 2019
@eladb eladb closed this as completed Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality package/awscl Cross-cutting issues related to the AWS Construct Library
Projects
None yet
Development

No branches or pull requests

2 participants