-
Notifications
You must be signed in to change notification settings - Fork 160
update godeps and use by makefile #231
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 godeps and use by makefile #231
Conversation
dcb08e9 to
7975741
Compare
|
I'm fine to update this. |
|
Yeah, let's get the other ones in. |
bc4a30a to
148ae00
Compare
|
@opencontainers/runtime-tools-maintainers |
.travis.yml
Outdated
| sudo: false | ||
|
|
||
| before_install: | ||
| - go get github.com/tools/godep |
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 this addition? Does Travis start using godep somewhere?
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.
godep will be used in 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.
Ah, I'd missed that between all the Godeps removals and vendor additions.
7ae267b to
1168ef5
Compare
da8931f to
4c7392f
Compare
|
As #349 is landed, can we start to review this PR? |
|
ping @liangchenye @mrunalp @hqhq |
4c7392f to
a01adbc
Compare
|
Can you skip this in Makefile? @Mashimiao |
a01adbc to
662d87e
Compare
|
@liangchenye yes, updated. I think there is no need to exit with output of golint |
|
@Mashimiao What tool are you using? Just godep or tools like vndr? Why there's no config file for dependencies? |
|
Just use godep. sorry for leave the config behind. |
| }, | ||
| { | ||
| "ImportPath": "golang.org/x/sys/unix", | ||
| "Rev": "f3918c30c5c2cb527c0b071a27c35120a6c0719a" |
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 do you need to add this in this PR?
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.
automatically added by godep, not sure if it should be here. But as unix also is called and depended by runtime-tools, I think may be it's OK
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 find runc or image-tools also add them into vendor directory
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.
They use x/sys/unix to replace syscall package, maybe it's used by the updated vendors, it's fine if is automatically added.
2dc42f7 to
89178d7
Compare
eb6c0c7 to
c7c0082
Compare
Makefile
Outdated
|
|
||
| .govet: | ||
| go vet -x ./... | ||
| OUT=$$(go vet ./... | grep -v "exit status" | grep -v vendor); if test -n "$${OUT}"; then echo "$${OUT}" && exit 1; fi |
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 would do something like:
packages = $(shell go list ./... | grep -v vendor)
.govet:
go vet $(packages)
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.
updated, PTAL
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.
Not exactly what I expected, but we can do that later.
remove test files in dependencies add directory vendor as _workspace will be deprecated in go1.8 Signed-off-by: Ma Shimiao <[email protected]>
As _workspace is deprecated and support for it will be removed when go1.8 is released. I think RUNTIME_TOOLS_LINK will not be needed any more Signed-off-by: Ma Shimiao <[email protected]>
Signed-off-by: Ma Shimiao <[email protected]>
Signed-off-by: Ma Shimiao <[email protected]>
c7c0082 to
7c5ce64
Compare
1 similar comment
We used to use this option but it's reverted in opencontainers#231 , not sure about the reason, it's simpler than to use the OUT hack. Signed-off-by: Qiang Huang <[email protected]>
Generated with: $ godep save ./... $ git mv Godeps/_workspace/src/github.com/xeipuuv/ vendor/github.com/ I'm not sure why I need the 'git mv ...'. We've been keeping our vendored content there since aab108d (update godeps and use by makefile, 2017-03-24, opencontainers#231), but my godep is still trying to save the content under Godeps/_workspace. Maybe it's because my system Go is still 1.7.4? Signed-off-by: W. Trevor King <[email protected]>
update dependencies to the latest version
and add directory vendor as _workspace will be deprecated in go1.8
Signed-off-by: Ma Shimiao [email protected]