Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Allow use parameters of type List<> in resources with properties where PrimitiveItemType = String and Type = List #321

Closed
wants to merge 1 commit into from

Conversation

xrn
Copy link
Contributor

@xrn xrn commented Oct 11, 2020

Issue #, if available:
#306 #282

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@xrn
Copy link
Contributor Author

xrn commented Oct 11, 2020

Like I mentioned in Issue - if we have PrimitiveItemType = String and Type = List - we should be able to use Parameters with type this means Ref::ParameterX (it is string) as well as []string{"A","B","C"}

Example

"AWS::RDS::DBCluster": {
      ...
      "VpcSecurityGroupIds": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbcluster.html#cfn-rds-dbcluster-vpcsecuritygroupids",
          "DuplicatesAllowed": false,
          "PrimitiveItemType": "String",
          "Required": false,
          "Type": "List",
          "UpdateType": "Mutable"
        }

@PaulMaddox will be awesome if you can take a look

@xrn xrn changed the title Allow use parameters of type List<> Allow use parameters of type List<> in resources with properties where PrimitiveItemType = String and Type = List Oct 11, 2020
@PaulMaddox
Copy link
Contributor

I think there's a pretty big downside to this approach - the API of goformation becomes less typed, more error prone for consumers, and harder to use. It shifts validation from compile time to runtime which is not a good thing. The more places we use interface{}, the less valuable this library is.

I think there is an opportunity to do something better here, and although it's a much bigger change, it fixes a lot of current issues with goformation...

One of the current issues with this library is that all structs accept properties by value. We need to move the libary to use pointers instead, which will fix issues such as #294, #181, #61 etc (there's more similar issues).

I think this would be the right opportunity to also include some smarter types that could allow the API of this library to retain it's strong typing, while allowing scenarios such as this issue.

@xrn xrn closed this Oct 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants