-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Update contributing guide build steps #1801
Conversation
.github/CONTRIBUTING.md
Outdated
- Set your `GOPATH` and `PATH` variable to be set to `~/go` via: | ||
|
||
```bash | ||
$ export GOPATH=~/go |
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.
Could you remove $
to facilitate copy-paste?
.github/CONTRIBUTING.md
Outdated
$ go generate | ||
|
||
# Standard go build | ||
$ go build ./cmd/traefik |
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.
Could you remove $
to facilitate copy-paste?
.github/CONTRIBUTING.md
Outdated
- This will allow your `GOPATH` and `PATH` variable to be set to `~/go` via: | ||
- Set your `GOPATH` and `PATH` variable to be set to `~/go` via: | ||
|
||
```bash |
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.
could you remove the \t on all the lines?
.github/CONTRIBUTING.md
Outdated
|
||
- Verify your environment is setup properly by running `$ go env`. Depending on your OS and environment you should see output similar to: | ||
|
||
```bash |
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.
could you remove the \t on all the lines?
.github/CONTRIBUTING.md
Outdated
$ export PATH=$PATH:$GOPATH/bin | ||
``` | ||
|
||
> Note: You will want to add those 2 export lines to your `.bashrc` or `.bash_profile` |
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.
could you remove the \t?
.github/CONTRIBUTING.md
Outdated
``` | ||
|
||
###### Build Træfik | ||
Once your environment is set up and the Træfik repository cloned you can build Træfik. You need get `go-bindata` once to be able to use `go generate` command as part of the build. The steps to build are: |
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.
could you add an empty line between title and content?
.github/CONTRIBUTING.md
Outdated
This can be verified via `$ go env` | ||
- You will want to add those 2 export lines to your `.bashrc` or `.bash_profile` | ||
- You need `go-bindata` to be able to use `go generate` command (needed to build) : `$ go get github.com/jteeuwen/go-bindata/...` (Please note, the ellipses are required) | ||
You will find the Træfik executable in the ```~/go/src/github.com/containous/traefik``` folder as ```traefik```. |
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.
could you replace ``` by only one `
updated. Thanks |
@@ -32,15 +32,48 @@ traefik* | |||
|
|||
- You need `go` v1.8+ | |||
- It is recommended you clone Træfik into a directory like `~/go/src/github.com/containous/traefik` (This is the official golang workspace hierarchy, and will allow dependencies to resolve properly) | |||
- This will allow your `GOPATH` and `PATH` variable to be set to `~/go` via: | |||
- Set your `GOPATH` and `PATH` variable to be set to `~/go` via: |
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 thought I had made the following comment already, but I can't see it.)
Given that we require Go 1.8 which doesn't need a set GOPATH
variable anymore, is this part still relevant?
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 agree 👍
Maybe we could mention the fact that it is possible to defined a custom GOPATH.
Maybe add this link: https://golang.org/doc/go1.8#gopath
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 found that I still need to set the path manually other wise I get generate.go:7: running "go-bindata": exec: "go-bindata": executable file not found in $PATH
:
jsturtevant:traefik (master)$ go get github.com/jteeuwen/go-bindata/...
jsturtevant:traefik (master)$ go generate
generate.go:7: running "go-bindata": exec: "go-bindata": executable file not found in $PATH
jsturtevant:traefik (master)$ go version
go version go1.8.3 linux/amd64
adding the two exports fix it. Maybe I have something configured wrong?
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.
Strictly speaking, you only need to set PATH
to find go-bindata
, or call it directly from the known path.
Practically speaking, you're probably right because most people define their go get
ed binaries in terms of the GOPATH
variable.
So I'm okay with this. 👍
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 now... good work thanks!
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
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
Description
Update to the docs to clarify the build steps. It took longer than I would like to admit to get build working because I missed a step and also setup glide when it wasn't needed (i wasn't changing dependencies).
I updated the docs to help clarify the steps. I hope you find this helpful and open to suggestions if I missed something.