Makefile lint target?

[simark]

Could we add a "make lint" target that would run cpplint.py with the right parameters? It could also run pep8/flake8 for Python, and others for other languages.

[nomis52]

That's a great idea.

We may also want to consider git pre-commit hooks to prevent untidy code from being submitted.

[peternewman]

@Keeper-of-the-Keys This is a bare minimum prerequisite for dropping linting of generated files, if you're feeling keen, given you seem to keep mentioning it.

[Keeper-of-the-Keys]

Cool, will see what I can do.

[peternewman]

@stefansiegfried do you fancy tacking this one in another PR too?

Our cpplint.py config is already in Travis.

[stefangs0x90]

@peternewman I would be happy to take this on

[stefangs0x90]

@peternewman any thoughts on moving the cpplint command from .travis-ci.sh into its own bash script?

/cpplint.py \
    --filter=-legal/copyright,-readability/streams,-runtime/arrays \
    $(find ./ \( -name "*.h" -or -name "*.cpp" \) -and ! \( \
        -wholename "./common/protocol/Ola.pb.*" -or \
        -wholename "./common/rpc/Rpc.pb.*" -or \
        -wholename "./common/rpc/TestService.pb.*" -or \
        -wholename "./common/rdm/Pids.pb.*" -or \
        -wholename "./config.h" -or \
        -wholename "./plugins/*/messages/*ConfigMessages.pb.*" -or \
        -wholename "./tools/ola_trigger/config.tab.*" -or \
        -wholename "./tools/ola_trigger/lex.yy.cpp" \) | xargs)
  if [[ $? -ne 0 ]]; then
    exit 1;
  fi;

Otherwise, it seems a bit cumbersome to do from the Makefile itself

[peternewman]

I'd probably rather not ship yet another .sh file if we can help it, it also feels a bit clunky to me if we have to do that.

I think you can probably drop the exit code check, I believe any non-zero code should stop the make command (older cpplint used to return the number of errors as the exit code, newer ones just exit 1 anyway I believe).

Ideally you can do some magic within the Makefile to replace all the find stuff given I'd assume we can get a list of files already as we're compiling the code and we can probably find some clever way to filter these bits out too (e.g. tagging them into custom groups which get included).

If it really isn't working have a look at this as an alternative option: https://github.com/OpenLightingProject/ola/blob/master/tools/ola_trigger/Makefile.mk

[stefangs0x90]

@peternewman I wanted to get in a preliminary implementation before Hacktoberfest ends. It still relies on the external script, but I will continue to iterate on this. If autotools makes the list of sources available somewhere that I can pass to cpplint, they have not made that information easy to find. Regarding the Makefile.mk you linked - was there a specific line(s) you could direct me to?

[peternewman]

@peternewman I wanted to get in a preliminary implementation before Hacktoberfest ends.

Heh, fair enough.

It still relies on the external script, but I will continue to iterate on this.

Great thanks.

If autotools makes the list of sources available somewhere that I can pass to cpplint, they have not made that information easy to find.

Yeah I'm not surprised. See my builtfiles kludge too for the kind of hoops you might have to jump through: https://github.com/OpenLightingProject/ola/blob/master/Makefile.am#L193-L196

Regarding the Makefile.mk you linked - was there a specific line(s) you could direct me to?

https://github.com/OpenLightingProject/ola/blob/master/tools/ola_trigger/Makefile.mk#L74-L80