build: make linter targets silent

[thefourtheye]

The linter targets are printing the commands they execute on screen.
This patch reduces the noise by not printing the commands.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

build

--

cc @nodejs/build

[bnoordhuis]

Doesn't bother me personally but LGTM.

[gibfahn]

I'd be somewhat -1 on this as I often copy the linter commands when I want to edit them, e.g. to add --fix and auto-fix linter errors. I think it's helpful for people running make lint to understand what's actually being run.

[thefourtheye]

@gibfahn We don't print the actual commands for many of the other targets. Even we don't print that for cpplint. As it is, I am getting the following in master

EXIT_STATUS=0 ; \
	/Library/Developer/CommandLineTools/usr/bin/make jslint || EXIT_STATUS=$? ; \
	/Library/Developer/CommandLineTools/usr/bin/make cpplint || EXIT_STATUS=$? ; \
	exit $EXIT_STATUS
./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules \
	  benchmark lib test tools
Total errors found: 0

The first line is definitely not necessary, as it is simply printing the target commands.

[aqrln]

@thefourtheye yes, the first line should definitely go away from the output :) Not sure about the actual commands for jslint and cpplint, though; I used to copy the eslint command too before it has carved to my memory well enough to be entered immediately without any extra mental overhead.

Even we don't print that for cpplint.

Hmm, I had never pay attention for it, but it is indeed inconsistent. Moreover, this output is somewhat confusing since the command comes from the jslint rule and the output from the cpplint one:

./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules \
	  benchmark lib test tools
Total errors found: 0

[refack]

what @gibfahn said (I don't know much about make but maybe you have a verbose option, maybe --debug=j)
Also what about vcbuild.bat

[gibfahn]

@gibfahn We don't print the actual commands for many of the other targets.

I think that for each distinct thing we do, we should print something to show we entered that section. Ideally you'd get a single line that you could copy to manually run that section. Then you'd get no other output unless there were errors/warnings. Consider someone running make test, that does about six things. For those of us who've spent hours poring over test logs it's usually pretty straightforward to work out where each output line comes from, but for a new Contributor it's confusing.

The aim is to balance not printing excessive info with not being too opaque IMO. I think tools/test.py and ninja -C do this really well, you get a continuously updating line so you know where you are (but no output is left behind), and then if there's an error you get the line it was on when it errored plus that error. I suspect that might be a little difficult for the Makefile though.

Even we don't print that for cpplint.

Then I think we should print the equivalent for cpplint (unless it's massively long).

The first line is definitely not necessary, as it is simply printing the target commands.

Agreed.

[thefourtheye]

@gibfahn I changed it to print the linter action being carried out. Now the result looks like this

➜  node git:(make-linter-silent) make lint
Running JS linter...
./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules \
	  benchmark lib test tools
Running C++ linter...
Total errors found: 0

[gibfahn]

Thanks!

[jasnell]

Landed in 716d831