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(task): quoting and command argument parsing #1467

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

iamhopaul123
Copy link
Contributor

fix #1466

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

@iamhopaul123 iamhopaul123 requested a review from a team as a code owner October 5, 2020 20:49
@iamhopaul123 iamhopaul123 requested a review from kohidave October 5, 2020 20:49
Copy link
Contributor

@huanjani huanjani left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Awesome work! Just a question.

@@ -18,7 +18,7 @@ Parameters:
ExecutionRole:
Type: String
Command:
Type: String
Type: CommaDelimitedList
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this require a documentation change? Like, is it backwards compatible if people just want to use double quotes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's backwards compatible. it's not a new change but just a bug fix i think.

Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Ship it!

@mergify mergify bot merged commit fc20492 into aws:mainline Oct 6, 2020
mergify bot pushed a commit that referenced this pull request Mar 4, 2021
In #1998, the way that the strings are split into string slice may cause bugs when passing the values to cfn template. This PR implements a similar fix as #1467.

Fixes #1998 

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
<!-- Provide summary of changes -->
fix aws#1466 
<!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
thrau pushed a commit to localstack/copilot-cli-local that referenced this pull request Dec 9, 2022
In aws#1998, the way that the strings are split into string slice may cause bugs when passing the values to cfn template. This PR implements a similar fix as aws#1467.

Fixes aws#1998 

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
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.

Quoting and command argument parsing for one-off tasks
3 participants