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 bash variable errors at workflow builder cmd #140

Merged
merged 1 commit into from May 2, 2018
Merged

Fix bash variable errors at workflow builder cmd #140

merged 1 commit into from May 2, 2018

Conversation

ghost
Copy link

@ghost ghost commented Apr 30, 2018

Fixes #139

  • Fix variable errors, use $
  • Only scan for *.wf.yaml files as archive may contain other *.yaml files (happened in real use case)
  • Fix parse call by removing wrong > redirection call

* Fix variable errors, use $
* Only scan for `*.wf.yaml` files as archive may contain other `*.yaml` files (happened in real use case)
* Fix parse call by removing wrong `>` redirection call
@erwinvaneyk
Copy link
Member

erwinvaneyk commented May 1, 2018

Thanks @thenamly for your contribution 😁 I noticed issues 1 and 3 too in #135, but it is great to have that in earlier. The *.wf.yaml issue I had not thought of before--nice idea to narrow the selection. Although the original intention was not to require files to have the .wf.yaml extension; it is more of a convention. We might want to change that later to a more generalized approach, for example with a blacklist or whitelist file to indicate which files to parse.

Let me check if there is an easy way for me to trigger the builds on our repo, instead of yours, to fix the travis tests (they currently require a gcloud api key, which you of course don't have).

@erwinvaneyk erwinvaneyk added this to the 0.3.0 milestone May 1, 2018
@erwinvaneyk
Copy link
Member

I am going to defer the fixing of the CI for external contributions to a later time. The YAML constraint is good--at least for now (see other comment). So LGTM 👍

@erwinvaneyk erwinvaneyk merged commit 1fd2acb into fission:master May 2, 2018
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.

1 participant