-
Notifications
You must be signed in to change notification settings - Fork 41
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
[#22] Makefile for Dunner #30
Conversation
Makefile
Outdated
GOTEST=$(GOCMD) test | ||
DEP=dep | ||
|
||
all : dep install |
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.
Let's add Phony Targets
See more on https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html
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.
Alright. 👍
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.
Added phony targets @agentmilindu, please check. 👍
Makefile
Outdated
dep: | ||
@$(DEP) ensure | ||
|
||
install: |
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 see some examples which use make install
to install the dependencies and make
to install the app. Shall we check how's the convention? My knowledge is limited on this.
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 will check and let you know and accordingly update this. Thanks. 👍
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.
@agentmilindu I couldn't find anything conclusive on this, but this approach makes sense to me,so I updated the commit accordingly. Please have a look, and as we move forward we will make changes as per requirements? I also added a clean target, so have a look at that as well. GNU make suggests some targets to be present in the makefile, so I added the ones which I thought made sense and were of use. 👍
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.
Cool
…s to make install for dependencies and make for software
…d its recipe in install instead
@agentmilindu @rehrumesh Please review the changes. Thanks. |
Works on #22
Added MakeFile for Dunner with dep and install under all and separate target for test. Please see if any changes are required. 👍