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

fix: Tags with value of False got evaluated to "" #3429

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Conversation

ConnorRobertson
Copy link
Contributor

Issue #, if available

#3426

Description of changes

When the value of a tag was False it would be set to the default "" value as the if statement evaluated the value rather than checking if it was None.

Description of how you validated changes

Tested by transforming template and looking at tags

Checklist

Examples?

Please reach out in the comments if you want to add an example. Examples will be
added to sam init through aws/aws-sam-cli-app-templates.

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

@ConnorRobertson ConnorRobertson requested a review from a team as a code owner November 22, 2023 01:05
Copy link
Contributor

@aaythapa aaythapa left a comment

Choose a reason for hiding this comment

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

I remember we had the PropogateTags property for some resources, will this error pop up for that property?

@ConnorRobertson
Copy link
Contributor Author

PropogateTags

This is a good question. I will investigate.

@ConnorRobertson
Copy link
Contributor Author

This same issue would have appeared in PropagateTags as well but this fix as is fixes that.

@ConnorRobertson ConnorRobertson merged commit bcd8238 into develop Nov 22, 2023
7 checks passed
@ConnorRobertson ConnorRobertson deleted the fix-tags branch November 22, 2023 17:40
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