-
Notifications
You must be signed in to change notification settings - Fork 51
Move from Make to Task #238
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
base: main
Are you sure you want to change the base?
Conversation
54bc9fc to
8715f36
Compare
113b64f to
77b3f30
Compare
232b789 to
9ae1ace
Compare
9ae1ace to
7a1dce7
Compare
|
|
||
| vars: | ||
| ARTIFACT_DIR: "{{.PWD}}/artifacts" | ||
| DEPLOYMENT_ID: '{{.DEPLOYMENT_ID | default "devex-cicd"}}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this defined and some other variables with the same definitions multiple times (and sometimes not used). Is that intentional? If not would it be better to move share variables into Taskfile.yml at the top level?
| - | | ||
| OS=$(uname) | ||
| if [ "$OS" = "Darwin" ]; then | ||
| curl -L {{.MAC_RPK}} -o {{.ARTIFACT_DIR}}/tmp/rpk.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just my two cents, I find it better to put non-local variables into the scope of the task so it's clear what is local to this task and what's expected to be shared.
a258733 to
78c4e69
Compare
When run during the connect test without this particular method of testing for the existence of keys the connect test fails.
78c4e69 to
7d3f336
Compare
Updates various clients and tooling in the repo and moves us to task from make to bring the repo into alignment with the k8s operator.