-
Notifications
You must be signed in to change notification settings - Fork 398
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
release in one step leveraging goreleaser #521
Conversation
f3369de
to
2277222
Compare
Makefile
Outdated
version: | ||
@echo "==> determining doctl version" | ||
@echo "" | ||
@ORIGIN=$$ORIGIN scripts/version.sh |
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 syntax didn't work for me during my local testing, I had to use,
@ORIGIN=${ORIGIN}
And then it all worked great.
Makefile
Outdated
_bump_and_tag: _install_sembump | ||
@echo "==> BUMP=${BUMP} bumping and tagging version" | ||
@echo "" | ||
@ORIGIN=$$ORIGIN scripts/bumpversion.sh |
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.
Same here, I had to use,
@ORIGIN=${ORIGIN}
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 did quite a bit of testing, and I feel like this is good to go, especially considering it'll you or me doing the first few releases so if any issues come up, we'll be capable fo debugging them.
My only feedback is about the Makefile syntax for passing the ORIGIN
arg to the scripts, other than, it works great! I was able to push a release to GitHub and Dockerhub and have them both work, and have the correct version strings.
Dockerfile.goreleaser
Outdated
@@ -0,0 +1,9 @@ | |||
FROM alpine:3.8 | |||
|
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.
Adding,
RUN apk add --no-cache curl
here fixes the issue with missing certs, from my testing at least.
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.
really? That's great!
5919331
to
f52a3f5
Compare
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.
LGTM!
see also #516
Removed dockerhub integration, will submit that separately
CODE NOW RELEASES TO PROD TARGETS!