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

Make schema for args of Cloud Build steps a list #2101

Closed
wants to merge 1 commit into from

Conversation

nownabe
Copy link
Contributor

@nownabe nownabe commented Sep 23, 2018

Because strings.Split cannot handle string arguments including space.

For example, "git commit -m "New Commit""

Thanks.

cf. https://godoc.org/google.golang.org/api/cloudbuild/v1#BuildStep

Because strings.Split cannot handle string arguments including space.

For example, "git commit -m \"New Commit\""

cf. https://godoc.org/google.golang.org/api/cloudbuild/v1#BuildStep
@paddycarver
Copy link
Contributor

Hi! Thanks for the PR. I see what you're going for here, but sadly, we're unable to merge this change until 2.0.0, as it's a breaking change. If you're comfortable waiting for that, that's awesome. If not, an interim solution may be to look into a more flexible parsing approach for the string that's able to take quotes into consideration.

@nownabe
Copy link
Contributor Author

nownabe commented Sep 27, 2018

Hi @paddycarver ! Thank you for your reply! 😄

I understood. I would like to think about another parsing approach. I have a branch to change another point of cloudbuild trigger, so I'll make a PR of this problem after that.

@paddycarver paddycarver modified the milestone: 2.0.0 Sep 30, 2018
@paddycarver paddycarver self-assigned this Oct 30, 2018
paddycarver added a commit to GoogleCloudPlatform/magic-modules that referenced this pull request Nov 27, 2018
Right now, cloudbuild trigger's args field expects a string, then splits
that string on spaces to get the list the API wants. This doesn't do
proper quote escaping, however, and creates problems, which led to
hashicorp/terraform-provider-google#2101.

This change turns it into a list instead of a string, so the user can
take care of the parsing and escaping themselves.

This is a breaking change, and should only go out with 2.0.0. We talked
about the transition paths available to us, and this seemed the most
reasonable.
@rileykarson
Copy link
Collaborator

Superseded by GoogleCloudPlatform/magic-modules#1145

@rileykarson rileykarson closed this Jan 4, 2019
@ghost
Copy link

ghost commented Feb 4, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Feb 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants