Included src-d/ci/Makefile.main in Makefile #76
Conversation
Codecov Report
@@ Coverage Diff @@
## master #76 +/- ##
=======================================
Coverage 85.03% 85.03%
=======================================
Files 16 16
Lines 942 942
=======================================
Hits 801 801
Misses 83 83
Partials 58 58Continue to review full report at Codecov.
|
.gitignore
Outdated
| @@ -1 +1,3 @@ | |||
| .linguist | |||
| .ci | |||
| Makefile.main | |||
There was a problem hiding this comment.
I have it in my local:
➜ enry.v1 git:(fix/makefile-ci) ✗ hexdump -c .gitignore
0000000 . l i n g u i s t \n . c i \n M a
0000010 k e f i l e . m a i n \n
000001cbut it seems that GH remove the new line
There was a problem hiding this comment.
weird, it seems correct now
cli/enry/main.go
Outdated
| usage: %[1]s <path> | ||
| %[1]s [-json] [-breakdown] <path> | ||
| %[1]s [-json] [-breakdown] | ||
| usage: enry <path> |
There was a problem hiding this comment.
what's the reason for hardcoding the binary name now?
There was a problem hiding this comment.
No reason, let me change it to the no hardcoded version
Makefile
Outdated
| LOCAL_TAG := $(shell git describe --tags --abbrev=0) | ||
| LOCAL_COMMIT := $(shell git rev-parse --short HEAD) | ||
| LOCAL_BUILD := $(shell date +"%m-%d-%Y_%H_%M_%S") | ||
| LDFLAGS = -s -X main.version=$(LOCAL_TAG) -X main.build=$(LOCAL_BUILD) -X main.commit=$(LOCAL_COMMIT) |
There was a problem hiding this comment.
minor: it may be better to use += or ?= instead of =, since having a customizable LDFLAGS by the user may be desirable.
There was a problem hiding this comment.
Maybe it's better just rename the variable to LOCAL_LDFLAGS because in the Makefile.main could be in the future a LDFLAGS to build the binaries for releases and it shouldn't be changed
what do you think?
|
|
||
| $(MAKEFILE): | ||
| @git clone --quiet $(CI_REPOSITORY) $(CI_FOLDER); \ | ||
| cp $(CI_FOLDER)/$(MAKEFILE) .; |
There was a problem hiding this comment.
If you just need the Makefile.main file, it may be simpler and more efficient to just get it:
wget https://raw.githubusercontent.com/src-d/ci/master/Makefile.main
There was a problem hiding this comment.
It's how the example does it https://github.com/src-d/ci/blob/master/examples/ci/Makefile
Should I change it anyway?
There was a problem hiding this comment.
hmmm, I'd like to hear what @smola or @erizocosmico have to say then, since they'll know better.
There was a problem hiding this comment.
It could be done with wget, Makefile.main has no other dependencies afaik. Check in with @mcuadros first though and if he approves then also make a PR in the examples.
There was a problem hiding this comment.
This is being handled here: src-d/ci#37
mcarmonaa
force-pushed
the
fix/makefile-ci
branch
from
a86894c to
40de88c
Compare
mcarmonaa
force-pushed
the
fix/makefile-ci
branch
from
40de88c to
370c13f
Compare
mcarmonaa
force-pushed
the
fix/makefile-ci
branch
from
370c13f to
8e0a26f
Compare
|
I opened a PR to discuss about using wget to download only Makefile.main |
#74