-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Use Golang built-in coverage test suites. #1127
Conversation
Signed-off-by: Arief Rahmansyah <ariefrahmansyah@hotmail.com>
-e examples \ | ||
-e scripts \ | ||
-e swagger-gen \ | ||
-e thrift-gen\ |
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.
this is not equivalent, glide only returns top-level pkgs
$ glide novendor
./swagger-gen/...
./crossdock/...
./cmd/...
./plugin/...
./storage/...
./thrift-gen/...
./examples/...
./model/...
./pkg/...
.
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.
Well, they are returning different result. But once they go to go test $TOP_PKGS
, the test results are same.
There are two options I have in mind:
- Keep using glide novendor
- Rename
TOP_PKGS
toTEST_PKGS
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 certainly want to stop using glide for that, I'm just concerned that there are 8 occurrences of TOP_PKGS
in the Makefile, so we need to ensure that changing what it refers to is still equivalent for all other built steps using it.
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.
Got it. I understand your concern.
TOP_PKGS
is used by 4 Go tools: go list
, go test
, go vet
, and golint
. And 2 other tools: gosec
and gosimple
. All of Go tools have the same behavior on handling packages argument. I also read gosec
and gosimple
documentation and the way they handle packages argument is same as Go tools.
Reading documentation is not enough, so I created a simple script to compare them. Please check it here: https://gist.github.com/ariefrahmansyah/89cecbd7f933f80a9258c0028c92a2f3
I'm sure that migrating from glide novendor
to go list
is fine 🙂
By the way, another thing to discuss, maybe we can merge TOP_PKGS
to ALL_PKGS
🤔
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.
By the way, another thing to discuss, maybe we can merge TOP_PKGS to ALL_PKGS
yes, I think it would make sense, I never liked the split, it was only there because some tools worked differently.
@ariefrahmansyah do you want to try again? My main concern is how to deal with packages where we explicitly have
|
@yurishkuro hi, yes i will find a time this week to try this again. thanks |
Which problem is this PR solving?
Short description of the changes