ThrowTheSwitch / Unity

Simple Unit Testing for C
ThrowTheSwitch.org
MIT License
3.98k stars 966 forks source link

Formalize Coding Standard and Check in CI #194

Closed mvandervoord closed 7 years ago

mvandervoord commented 8 years ago

It's time to formalize our coding style and then verify people are matching it from rake

xor-gate commented 8 years ago

Checklist:

Nice to have in CI (has nothing todo with codestyle, but with quality):

The checker tools should fail with exit code != 0 so Travis builds (and PR checks) will fail. So we enforce a codestyle for everybody and all contributions are the same.

I have googled many times for a C codestyle checker (firmware) and the best match was the checkpatch.pl tool because it is most complete. It needs some tweaking for some firmware software but it is better then splint. Tools like scan-build are very usefull to have in travis also.

I'm not 100% sure what you mean with matching it from rake.

mvandervoord commented 8 years ago

We use rake to drive all the CI self-test process. So I was just commenting that I'll need to put whatever tool is selected into the CI process.

It probably won't be checkpatch.pl. There isn't really a good reason to pull in a second full scripting language into the project requirements, particularly just for codestyle validation.

Very likely it'll be something like AStyle since it has good cross-platform support already.

xor-gate commented 8 years ago

You are right I didn't know about the astyle --formatted option where you can verify if a file is changed. We should loop then over the C source files and check if any output is generated.

mvandervoord commented 8 years ago

:) awesome. We're on the same page with that one.

xor-gate commented 8 years ago

Odd thing with AStyle currently is when --formatted is given and one ore more files has changed the exit code is still zero. Then we need to hack something to check the output if it contains Formatted <file> with grep or something else.

mvandervoord commented 8 years ago

that's fine. rake can easily parse the output... we're already doing that with other test tools

xor-gate commented 8 years ago

Nice, i'm clueless with rake. I only have experience with CMake. Should we also integrate valgrind and scan-build for travis ? Or is this a little overkill.

dpostorivo commented 7 years ago

If you are still looking for a ruby linting tool, rubocop is really good and is being actively developed on.

wolf99 commented 7 years ago

Another option, the exercism xc repo currently uses is to use indent on a source file and then checks the result by doing a quiet diff against the indent result... See here: https://github.com/exercism/xc/blob/master/bin/verify-indent

mvandervoord commented 7 years ago

Finally added a coding standard to build off of.

xor-gate commented 7 years ago

To bad its not written in RestructuredText or markdown. A PDF (== binary) is IMHO not to way to go and add to a source-file repo. Is there any reason? Even the linux kernel is switching from AsciiDoc to RestructuredText (https://github.com/torvalds/linux/blob/master/Documentation/process/coding-style.rst).

mvandervoord commented 7 years ago

Momentum. All our documentation is in pdf's.

Maybe you want to open a separate issue to suggest converting all our documentation to markdown. I don't disagree with your point. :)

xor-gate commented 7 years ago

You are right, thats offtopic. Glad to see some codestyle getting pushed out for ThrowTheSwitch!

dpostorivo commented 7 years ago

There is actually a well known ruby style guide https://github.com/bbatsov/ruby-style-guide which what you listed is already goes along with it and rubocop which I mentioned before would enforce it and can be tailored to what we want and don't want and we could use it with the CI pretty easily.

mvandervoord commented 7 years ago

:) swank

dpostorivo commented 7 years ago

Just as a preview, I set the default up on a commit a while ago. I didn't suggest it as a pull request because I didn't know if that's what was wanted, but it will give you an idea of the default configuration and "offenses" currently in the repo : https://github.com/dpostorivo/Unity/commit/08867fb6aec94847e42fa7bddd4517ec0006b1e9

mvandervoord commented 7 years ago

@dpostorivo and @xor-gate, have I mentioned how awesome you people are? Thanks for all the ideas... and even more for the sample code!

mvandervoord commented 7 years ago

I'm really liking Rubocop and have started to merge it into Unity. I'll be doing the same to the other projects once I get further along with this one.

dpostorivo commented 7 years ago

@mvandervoord cool. Let me know if you need any help.