-
Notifications
You must be signed in to change notification settings - Fork 60
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
Use the released CLI artifacts #204
base: main
Are you sure you want to change the base?
Conversation
The version can be bumped automatically using sed: $ sed -i '' 's/^CLI_VER.*/CLI_VER := v0.13.0/' Makefile |
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.
This seems fine to me, superficially. One suggestion for picking up the latest CLI release version below. I'll leave it to others more familiar with this to give an approval.
This should also fix #107 |
Also makes progress on #43 |
@GOOS=linux GOARCH=$* CGO_ENABLED=$(CGO_ENABLED) make -C $(TCTL_ROOT) build | ||
@cp ./$(TCTL_ROOT)/tctl build/$*/ | ||
@cp ./$(TCTL_ROOT)/tctl-authorization-plugin build/$*/ | ||
@gh release -R temporalio/cli download $(CLI_VER) -p "*linux_$(*).tar.gz" -O - | tar --to-stdout -z -x -v -f - temporal > build/$*/temporal |
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.
@gh release -R temporalio/cli download $(CLI_VER) -p "*linux_$(*).tar.gz" -O - | tar --to-stdout -z -x -v -f - temporal > build/$*/temporal | |
@curl -L https://github.com/temporalio/cli/releases/download/v$(CLI_VER)/temporal_cli_$(CLI_VER)_linux_$(*).tar.gz | tar --to-stdout -z -x -v -f - temporal > build/$*/temporal |
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.
Preferable to using gh
imho.
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.
Why's that?
Re-thinking this, I think we should build for temporaliotest and releases differently. Release builds should use released assets rather than building (action can prompt for the various release numbers). Test builds should just use whatever the submodules are at. We don't have the complex docker tags for test builds, so we don't have to calculate any tags. |
What was changed
We use the pre-built CLI artifacts from the official releases rather than compiling them ourselves
Why?
Calculating the CLI's version based on the tag is a pain (see #203) and I don't want to do that in our Makefile, so we're going to download the appropriate artifact from the official release instead
Note
This is gross but it works. I'm putting this up to stimulate discussion on how we should do it.