-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Improve govendor testing #1623
Improve govendor testing #1623
Conversation
Use `govendor list +outside +unused` for finding missing or unused deps and govendor status for catching modified vendor.
This should failed at CI and be ok after #1620. So we could merge it after. |
@sapk but seems CI is not failed. |
@lunny I need to count line of |
@@ -1,6 +1,6 @@ | |||
{ | |||
"comment": "", | |||
"ignore": "test", | |||
"ignore": "test appengine", |
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 was this added?
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.
Because we have un-vendored deps relative to appengine. I think that it's a sub deps but I haven't found/search for it. I don't even know if we (plan to) support appengine. So this could be a good place to discuss about it ^^
Makefile
Outdated
@@ -84,7 +84,13 @@ test-vendor: | |||
@hash govendor > /dev/null 2>&1; if [ $$? -ne 0 ]; then \ | |||
go get -u github.com/kardianos/govendor; \ | |||
fi | |||
govendor status +outside +unused || exit 1 | |||
govendor list +unused | tee /tmp/wc-gitea-unused |
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.
Please use $TMPDIR
instead (same for the next command-block)
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.
The var TMPDIR (or TEMPDIR) seems to not be set when I run it via drone exec
. I will add TMPDIR := $(shell mktemp -d)
LGTM |
LGTM |
@bkcsoft waiting for your confirm. |
Use
govendor list +outside +unused
for finding missing or unused deps andgovendor status
for catching modified vendor.Ref : #1620