Skip to content

Fill the docker section - #42

Open
nhurel wants to merge 3 commits into
campoy:masterfrom
nhurel:docker
Open

Fill the docker section#42
nhurel wants to merge 3 commits into
campoy:masterfrom
nhurel:docker

Conversation

@nhurel

@nhurel nhurel commented Aug 4, 2017

Copy link
Copy Markdown

Proposal for your issue #5

@googlebot

Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@nhurel

nhurel commented Aug 4, 2017

Copy link
Copy Markdown
Author

I signed it!

@googlebot

Copy link
Copy Markdown

CLAs look good, thanks!

@campoy campoy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hey, thanks so much for this! I sent a couple of comments, I'll send more once you've done these edits.

Comment thread 2-building-artifacts/2-docker.md Outdated
- Local development

### Continuous integration
When using a CI server like Jenkins, it can become cumbersome to have to install all the go versions required by your different projects. And even more if you use your CI server to build projects in other languages, each with its own version (java8, java7, node, ruby, python, ...). For this use case, it can be easier to build the projects in docker containers. An example script could be :

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

please break this into lines for easier review

Comment thread 2-building-artifacts/2-docker.md Outdated

### Continuous integration
When using a CI server like Jenkins, it can become cumbersome to have to install all the go versions required by your different projects. And even more if you use your CI server to build projects in other languages, each with its own version (java8, java7, node, ruby, python, ...). For this use case, it can be easier to build the projects in docker containers. An example script could be :
```

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

'''bash
$ docker ...

Comment thread 2-building-artifacts/2-docker.md Outdated
### Continuous integration
When using a CI server like Jenkins, it can become cumbersome to have to install all the go versions required by your different projects. And even more if you use your CI server to build projects in other languages, each with its own version (java8, java7, node, ruby, python, ...). For this use case, it can be easier to build the projects in docker containers. An example script could be :
```
docker run --rm -v $PWD:/go/src/github.com/repo/project -w /go/src/github.com/repo/project golang:1.8.1 go build -o dist/binary

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why the -o dist/binary?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

because copy/pasting from another script 😇 I'll remove the option

This will run the `go build` command with go version 1.8.1, sharing your current directory with the container. Obviously, you can run `go fmt/vet/test/...` commands the same way

### Cross building binary
Usually, cross building a binary requires to install go from sources. Using the golang image, you can install the binary distribution on your computer for day-to-day use and rely on the docker image to build binaries for different targets.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is wrong, you can cross compile by installing Go with an installer to install from source

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

OK, I don't know how to do that TBH. Maybe my use case of using the golang image to generate binary is not a good example and you want to drop it ?

```
This will run the `go build` command with go version 1.8.1, sharing your current directory with the container. Obviously, you can run `go fmt/vet/test/...` commands the same way

### Cross building binary

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I expect this section to talk about how you can do

$ GOOS=linux go build
$ docker build .

Where Dockerfile adds the linux binary

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

hmmm... sorry don't understand what you mean here. The use case I have in mind in this section is only to use docker run to generate a binary, nothing more. Building an image by adding the binary in the Dockerfile is shown as an example in the sections below.

add bash syntax highlighting to command scripts
@nhurel

nhurel commented Aug 11, 2017

Copy link
Copy Markdown
Author

I've integrated part or your remarks. I don't fully understand what you expect from the Cross building binary section, I let you read my comments.

@campoy

campoy commented Aug 23, 2017

Copy link
Copy Markdown
Owner

Hey, I just came back from holidays and I'll be reviewing this in a couple days. Thanks so much!

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.

3 participants