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 proxy build broken by global_dictionary.yaml #615

Closed
wants to merge 1 commit into from

Conversation

yangminzhu
Copy link
Contributor

@yangminzhu yangminzhu commented Aug 17, 2018

I made a comment in https://github.com/istio/api/pull/605/files but think it's faster to send out a PR directly.

@mandarjog
Line 316 - "-" break the proxy build as create_global_dictionary.py parse it to: ""-"", which is invalid in .cc file.

Could you suggest the proper way to fix this? Should we just remove the " or make it \"-\" or even update the script?

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: rshriram

If they are not already assigned, you can assign the PR to them by writing /assign @rshriram in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Aug 17, 2018
@mandarjog
Copy link
Contributor

@yangminzhu can you verify that the resulting file is still a valid yaml file?
There is a go generate command in istio/istio where mixer server generates go code from this.

@bianpengyuan You had faced an issue with -, and I thought we got rid of it.

@bianpengyuan
Copy link
Contributor

@mandarjog instead of removing it, I added some logic to escape " in list generation code in istio/istio#8087.

@yangminzhu
Copy link
Contributor Author

@bianpengyuan so I think the right way to fix is to also update create_global_dictionary.py to do the same escape? If so I'll update this PR as well. Thanks.

@bianpengyuan
Copy link
Contributor

@yangminzhu Yes I think so. Let's do that.

@yangminzhu yangminzhu closed this Aug 20, 2018
@yangminzhu yangminzhu deleted the fix_proxy_build branch March 1, 2019 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants