search

aws-cloudformation / cloudformation-resource-schema

The CloudFormation Resource Schema defines the shape and semantic for resources provisioned by CloudFormation. It is used by provider developers using the CloudFormation RPDK.
Apache License 2.0
90 stars 38 forks source link

Adding conditionalCreateOnlyProperties to metaschema #121

Closed ammokhov closed 3 years ago

ammokhov commented 3 years ago

Issue #, if available: https://github.com/aws-cloudformation/aws-cloudformation-resource-schema/issues/30

Description of changes:

Adding conditionallyCreateOnlyProperties to meta schema, so that resource providers could model properties that could not deterministically predict replacement/inline update

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

benkehoe commented 3 years ago

Could I suggest these be called conditionalCreateOnlyProperties? Adverbs in field names are pretty uncommon, and as a user I think I would often forget that it was conditionallyCreateOnlyProperties. It's a create only property that is conditional, rather than a property that is conditionally create only

aygold92 commented 3 years ago

Could I suggest these be called conditionalCreateOnlyProperties? Adverbs in field names are pretty uncommon, and as a user I think I would often forget that it was conditionallyCreateOnlyProperties. It's a create only property that is conditional, rather than a property that is conditionally create only

I at first thought to call it conditionalCreateOnlyProperties, but I'm not sure what the distinction is between "a create only property that is conditional" and "a property that is conditionally create only". Actually, it seems more like the second one in this case. I'd personally be fine calling it either one

benkehoe commented 3 years ago

My main point is that it seems unusually phrased, and thus easy to forget/get confused. But in my mind, it's a create-only property, that isn't always triggered, as opposed to be only sometimes create-only.

benkehoe commented 3 years ago

Thanks for making the change! I saw the merge notification, but not the new commit. When I talked with TAM about the new feature, I called it conditionallyCreateOnlyProperties and they echoed back conditionalCreateOnlyProperties, which I think is evidence that it would have caused confusion.