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

RFC 0009: Adding looping functionality in CFN Template (Fn::For) #8

Merged
merged 1 commit into from
May 2, 2022
Merged

RFC 0009: Adding looping functionality in CFN Template (Fn::For) #8

merged 1 commit into from
May 2, 2022

Conversation

mingujo
Copy link
Contributor

@mingujo mingujo commented Apr 27, 2022

Language Enhancement Request For Comment

This is a request for comments about a new Language Extensions intrinsic function Fn::For. See TRACKING_ISSUE for
additional details.


Licensing

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

@mingujo mingujo changed the title RFC 0001: Adding looping functionality in CFN Template (Fn::For) RFC 0009: Adding looping functionality in CFN Template (Fn::For) Apr 28, 2022
RFCs/0009-Fn::For.md Show resolved Hide resolved
RFCs/0009-Fn::For.md Show resolved Hide resolved
lejiati
lejiati previously approved these changes May 2, 2022
#### Use case: Replicate a single Resource

* In Json
```aidl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind removing these aidl things?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right. Thank you. Removed them

- ["Index", "Value"],
- "CollectionToIterate",
- "TemplateSnippet"
- "LogicalId"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we had briefly touched on it in a discussion, but I don't quite remember why we wouldn't put the LogicalId before the TemplateSnippet. The snippet will be the largest value whereas the Logicalid is just a short string. I think readability would be better if the LogicalId comes before the snippet. Is it because it's optional? Do you think it would be too confusing to have an optional parameter in the second to last position?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we briefly talked about placing LogicalId before TemplateSnippet. We discussed that it could be tricky for users since it's an optional parameter while parameters are positional, not named ones.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite follow the argument, but I'm ok putting the RFC out like this. I'm curious to hear from customers, if they think this parameter should go in the third position or last. I don't see a technical reason why that wouldn't be possible and I think it would improve readability (although I acknowledge that having an optional parameter that's not in the last position is unusual.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually optional parameters are placed at the end so they can be truly optional, but I can see arguments from the other side as well. Let see what customers think about this.

Copy link
Contributor

@lejiati lejiati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ship!

- ["Index", "Value"],
- "CollectionToIterate",
- "TemplateSnippet"
- "LogicalId"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually optional parameters are placed at the end so they can be truly optional, but I can see arguments from the other side as well. Let see what customers think about this.

@mingujo mingujo merged commit d1fcdc9 into aws-cloudformation:main May 2, 2022
@mingujo mingujo deleted the min/FnForRFC2 branch May 2, 2022 18:10
mingujo added a commit that referenced this pull request May 2, 2022
jlhood pushed a commit that referenced this pull request May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants