-
Notifications
You must be signed in to change notification settings - Fork 143
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
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
@@ -5,14 +5,15 @@ go: | |||
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 <mashimiao.fnst@cn.fujitsu.com>
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 <mashimiao.fnst@cn.fujitsu.com>
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
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 <h.huangqiang@huawei.com>
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 <wking@tremily.us>
update dependencies to the latest version
and add directory vendor as _workspace will be deprecated in go1.8
Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com