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 0014: New Intrinsic Function to Convert Template Block to JSON String #76

Merged
merged 9 commits into from
Jun 28, 2022

Conversation

mluk-aws
Copy link
Contributor

Language Enhancement Request For Comment

This is a request for comments about a new intrinsic function to convert template blocks to JSON strings. See #14 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.

Copy link
Contributor

@jlhood jlhood left a comment

Choose a reason for hiding this comment

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

Looks good. Just want to add syntax highlighting to examples. This is a good practice in general but for this feature in particular, it emphasize the syntax highlighting benefit of Fn::ToJsonString.

RFCs/0014-ToJsonString.md Outdated Show resolved Hide resolved
RFCs/0014-ToJsonString.md Outdated Show resolved Hide resolved
RFCs/0014-ToJsonString.md Outdated Show resolved Hide resolved
RFCs/0014-ToJsonString.md Outdated Show resolved Hide resolved
jlhood
jlhood previously approved these changes May 13, 2022
RFCs/0014-ToJsonString.md Outdated Show resolved Hide resolved
RFCs/0014-ToJsonString.md Outdated Show resolved Hide resolved
@benkehoe
Copy link

This is unrelated to the RFC itself, but I will say it in every forum available to me: no new resources should be allowed to ship that require string-containing-JSON, even if the underlying APIs require it (the resource handler should perform the conversion).

@iann0036
Copy link

Per offline conversation, I would suggest that instead of making this an explicit action, we simply refer to the specification / registry type to determine the appropriate type at runtime. If the provided input is an object but the spec expects a string, simply perform the transformation automatically.

@benkehoe
Copy link

determine the appropriate type at runtime

I'm skeptical of this. I would be fine if the type of these properties was "string containing JSON", but when it's just "string" I think coercing every string property value to string is removing the ability to lint and provide up-front errors, as you essentially eliminate any type errors for string-valued properties.

jlhood
jlhood previously approved these changes May 16, 2022
RFCs/0014-ToJsonString.md Outdated Show resolved Hide resolved
RFCs/0014-ToJsonString.md Outdated Show resolved Hide resolved
Co-authored-by: Malik <60721392+MalikAtalla-AWS@users.noreply.github.com>
@arthurboghossian arthurboghossian merged commit 3ae041a into aws-cloudformation:main Jun 28, 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.

7 participants