Closed SuperSandro2000 closed 4 years ago
Also, you didn't remove build.sla
and install.sh
.
Could you not add --no-print-directory, this is GNU make specific and users and tools might want/expect path to be printed. You are not using any syntax specific to bash, so you could replace SHELL by /bin/sh to allow building on a wider set of systems.
I just copied that from my default Makefile.
Changed all the things you requested.
Thanks for the quick fix!
You are missing a .PHONY
tag for the install
target and then it should be good.
@lyknode Added that
Ah, we forgot the clean
rule. Could you make it remove the generated manpage?
The manpage is currently tracked in git and removing it would cause changes to the cwd.
When the manpage is converted to troff we don't need the generation or clean.
As per last comment on #38, the manpage would be removed and the markdown kept. But you are right, the clean
target doesn't need to be added right now. It can be done afterward as the file is removed from the repo.
Hi @SuperSandro2000,
I was going to take care of that but you did it first ^^. Allow me to comment on some suggestions.
$(DESTDIR)
? Those are used by packaging tools, allowing files to be installed in a relative path from PREFIX. Usually in makefiles, you would find:test
andinstall
in theall
target? By convention,make
must only build and not require privileges (whichinstall
might depending onDESTDIR
andPREFIX
), ideally users domake && make test && sudo make install
.mgitstatus
with a 755 mode?--no-print-directory
, this is GNU make specific and users and tools might want/expect path to be printed.The next two suggestions are just my preference so fell free to ignore those totally:
SHELL
by/bin/sh
to allow building on a wider set of systems.DEFAULT_GOAL
if all is your first target in the makefile, which it is by convention.