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 build stuff #10

Open
wants to merge 3 commits into
base: 1.13_upgrade
Choose a base branch
from
Open

Fix build stuff #10

wants to merge 3 commits into from

Conversation

lwahlmeier
Copy link
Member

@thorix this includes your changes for 1.13

@@ -1,23 +1,9 @@
FROM golang:1.13-alpine as builder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the removal of this section due to the build script but I like the idea of keeping this part in there for other folks to have a nice way of building locally without having to run the full buildDocker.sh script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new version also requires you to build the binaries ahead of the Docker file which works fine for the pipeline but now forces people into a 2-step process when building locally.

skip_cleanup: true
on:
tags: true
- provider: script
script: bash scripts/publish_docker.sh ${TRAVIS_BRANCH}
script: bash scripts/publish_docker.sh ${TRAVIS_BRANCH} true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like publish_docker.sh was deleted?

echo "---------------------"

docker run --rm -v "$PWD":/go/va -w /go/va golang:1.13-alpine \
sh -c "apk add bash zip && ./build.sh ${VERSION} linux"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to call apk add bash zip twice here? Also, that means this script will only work on alpine machines (can't run this file on other flavors or macos)?

@@ -0,0 +1,14 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this file broken out so we can all build consistently.

cd bin
cp vadmin-${GOOS} vadmin
zip vadmin-${GOOS}-${VERSION}.zip vadmin
rm vadmin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe leave the binary for local dev?

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.

2 participants