[riot-notifications] [RIOT-OS/RIOT] uncrustify: add auto uncrustify with blacklist (#8519)

Kaspar Schleiser notifications at github.com
Fri Jan 25 16:30:16 CET 2019


> we can already run Vera++ _now_ without adding `/* INDENT-OFF */` tags and would check the whole repo as it is.

But it would output a lot of warnings, right?

> In fact, that's the way how I've been checking coding style on PRs.

Well, I'd like to get you (and every human) out of the loop. Even looking at warnings regarding whitespaces is a waste of time in 99% of the cases.

I'd like to arrive to the most simple ruleset regarding formatting:

1. in order to comply with our formatting run $tool with $config

That should be it. For the initiated, there'll be an option to disable formatting (for where it really helps or where $tool really messes up). Otherwise, $tool is authoritative, and changes to formatting must be PR'ed to $config.

It is very nice if Vera++ let's us know about line length issues and missing spaces, but if it is not the right tool to be used for *fully automatic reformatting* on the users end, why bother?

I don't care which tool does it in the end, I just don't want to be bothered but still have consistent code looks.

(Vera++ happens to not be packaged for my system (current Arch), and it seems like it needs more than one file for its configuration, so my personal preference is strongly towards uncrustify for simplicity's sake.)

> IMHO we should also run `uncrustify` in addition to Vera++ on the CI. Vera++ to provide a comprehensive list of quality defects and `uncrustify` to check if its output is still in line with Vera++'s assessment.

Maybe. But a very important aspect of reducing reviewing overhead is to have $tool that is *authoritative* on all things formatting. No discussion. No second opinion. Let's have whitespace questions be answered *fully automatic*.

(And honestly, I think we should avoid wasting time getting the tools in sync, which will also be a waste of time, if a simple tool with agreement on one configuration solves the problem, IMO...)



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/8519#issuecomment-457610429
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190125/d2500fc5/attachment.html>


More information about the notifications mailing list