Fix Makefile rules for integration tests #154
Conversation
dpordomingo
left a comment
dpordomingo
left a comment
There was a problem hiding this comment.
not a strong-strong-strong opinion... but I'd really really really prefer if we'd follow ci api if there is no strong reason to ignore it.
Makefile
Outdated
|
|
||
| OS := $(shell uname) | ||
|
|
||
| bin-clean: |
There was a problem hiding this comment.
In order to follow ci api, I think we should override ci/Makefile.main::clean target, instead of using a new one.
There was a problem hiding this comment.
then it complains about overriding (warning). If everybody prefers that, I'm okay to change.
There was a problem hiding this comment.
yes, and imo the overriding warning is something expected when working with Make. And it's also a good way to notify that the used clean is different from the common one.
There was a problem hiding this comment.
I also prefer using clean.
|
Would it be possible to change |
|
@carlosms taking into account that src-d/ci#98 is open for more than 4 months - no. |
|
Still, if it makes sense to include the fix in src-d/ci, we could submit a PR there, and while we wait for it to be merged override |
Fix #152 Go clean doesn't work with go modules and it doesn't make much sense to run it when modules are used. Ref: golang/go#31002 Because sourced-ce uses go modules for dependency resolution it is expected that GO111MODULE is enabled. This commit removes running `go clean` and executes `rm` only. Signed-off-by: Maxim Sukharev <max@smacker.ru>
|
repushed with override. @carlosms I don't see any sense in creating PRs that aren't geting merged ever. |
dpordomingo
left a comment
dpordomingo
left a comment
There was a problem hiding this comment.
LGTM, many thanks for considering my suggestions.
|
@dpordomingo overriding definitely makes sense. I just don't like warnings 😅 |
4 participants
Fix #152
Go clean doesn't work with go modules and it doesn't make much sense to
run it when modules are used.
Ref: golang/go#31002
Because sourced-ce uses go modules for dependency resolution it is
expected that GO111MODULE is enabled.
This commit removes running
go cleanand executesrmonly.Signed-off-by: Maxim Sukharev max@smacker.ru