-
Notifications
You must be signed in to change notification settings - Fork 874
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 for backtick escape error #358
Conversation
/assign @jlewi |
/assign @hougangliu |
@kkasravi Why use escaping at all? i.e. rather than embed YAML in go files; why not just create YAML files and have the unittests load those files? If this is a quick fix then doing that then I think its fine. Longer term though it seems like #306 (verifying expected values) is pushing us in the direction of creating a test_data directory that would contain YAML files representing the expected output of kustomize build. So storing YAML files as opposed to embedding YAML in go files seems more consistent with that. That said, there's probably a lot more changes needed to realize #306 and I'm not sure that changing the current scripts to emit the kustomize files to YAML rather than embedding in the go code really moves the needle. So I'm fine with the workaround in this PR. |
please resolve the conflict. lgtm |
82bed9e
to
c749132
Compare
thanks @jlewi - it would be more straight forward to store the files in a different directory and read them in. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlewi 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 |
* Quick fix in SDK documentation Takes some excerpts from Christian's website PR. I will be revisiting after release to figure out the overlap with User guide. * SDK Packages Overview (kubeflow#359) Co-authored-by: Christian Kadner <ckadner@us.ibm.com>
Which issue is resolved by this Pull Request:
Resolves #356
Description of your changes:
backticks are escaped as described in the issue.
a second fix is related to returning the earliest 'manifests' in a directory path rather than the latest. If a subdir ends with manifests you will get an error in unit test generation.
Checklist:
cd manifests/tests
make generate
make test
This change is