Add a Cppcheck step

[benallard]

No description provided.

[krf]

Heya. Sorry for chiming in -- I've just seen this PR through IRC.

This looks nice already, but I'd be glad if you could make this a bit more flexible:

• Add a source=["."] () parameter for defining the list of files or paths for cppcheck
• Add an extra_args=[] (similar to the SVN step's 'extra_args') where one can pass additional options to cppcheck

Makes sense? Other opinions?

[sa2ajj]

@krf do not be sorry, all feedback is welcome!

[benallard]

Glad you like it !

It's quite easy to override the complete command parameter. this way it allow you to also change the path to cppcheck, or specify only a few messages that you'd like to enable ...

If we want to add some special support for the other parameter, then we'd have to add support for those ones also ... I'm not sure where to draw the line between the extra_args, and the customization args ...

I'd be glad to implement it if we come to a sensible agreement !

[benallard]

The minimum I should do is to add in the doc that customization of the command parameter is possible.

[sa2ajj]

I think extra_args is a good first step.

Once we understand (by users' feedback) what common options are there, we could provide them as explicit parameters.

[benallard]

Should we put the --inconclusive as default for extra_args ?

I.e.: When someone overrides extra_args, and want inconclusive, he need to explicitly give it ?

[krf]

Well, the reason I'm proposing both is that command already contains a path: ".". You cannot get rid off that by just passing extra_args. source is the most important options to cppcheck, after all, thus I think it makes sense to parameterize that one.

[benallard]

What about enable=all, should it also be put as default for extra_args ?

[sa2ajj]

re inconclusive -- this seems to be a good candidate for user-facing options.

[benallard]

Decided to put my whole parameters as extra_args. The downside of this is that it is not possible to override the whole command by giving command as parameter.

[tardyp]

why dont you put those parameters as constructors params? with None as default.
Also, you probably want to put those params as renderables

[benallard]

Agreed on the renderables.

I will set the default for extra_args to [], and add a enable parameter (default to nothing: default for cppcheck), and a Boolean inconclusive parameter (default to False: don't set the param).

[tardyp]

beware it is not advised to put [] in default arguments of params, it preferable to default to None, and do

if p is None
   p = []

http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments

[gsemet]

+1

[tardyp]

👍 except for the weird optional params constructor.

[benallard]

Fixed the parameters. Should be fine now.

[benallard]

I don't think we have trouble with enable passed as renderable as the str of a renderable returns its content, so that it can be nested (like I do there). Or do I have to add a str(r) for r in self.enable in my join call ?

[benallard]

Anyway, test should tell ...

[benallard]

Added test for renderables, removed enable from the renderables

[tardyp]

👍

[benallard]

This is good to go, travis is (now) happy, just tell me if I forgot any comment.