Skip to content
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

build: cpplint should always run, even if jslint fails #12082

Closed
mscdex opened this issue Mar 27, 2017 · 2 comments
Closed

build: cpplint should always run, even if jslint fails #12082

mscdex opened this issue Mar 27, 2017 · 2 comments
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.

Comments

@mscdex
Copy link
Contributor

mscdex commented Mar 27, 2017

  • Version: all
  • Platform: all
  • Subsystem: build

As the title says, it would be nice to always run both jslint and cpplint, even if one or both exit non-zero.

@mscdex mscdex added build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests. labels Mar 27, 2017
@krydos
Copy link
Contributor

krydos commented Apr 7, 2017

@mscdex what do you think about implementation?

Is it ok to do something like this?

lint:
        EXIT_STATUS=0;\
	$(MAKE) jslint || EXIT_STATUS=$$?; \
	$(MAKE) cpplint || EXIT_STATUS=$$?; \
	exit $$EXIT_STATUS

Or it will be better to create additional script inside ./tools that will run list of passed make targets?
Code above looks a bit complex probably and it will be not so easy to extend.

In case of the script it can look something like this:

lint: 
   tools/lint_runner.sh jslint cpplint

and this bash script can remember exit status of each job and return non 0 in case one of them failed. It looks a bit more extendable and pretty. I'm just not sure if it is ok to create new files in tools directory

@mscdex
Copy link
Contributor Author

mscdex commented Apr 7, 2017

I'm ok with the first solution for Makefile, that's actually more or less how nodejs/github-bot does it. vcbuild.bat would need to be updated accordingly as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

2 participants