On 02/25/2016 08:05 PM, Martin Sebor wrote:
Recently I had the opportunity to learn about a number of rather
subtle style conventions sometimes enforced during code reviews
(though not inconsistently followed in GCC code). To help find
these kinds of problems before a patch is submitted and cut down
on the subsequent back-and-forth, I've added checks for some of
these conventions to the check_GNU_style.sh script. In addition,
since the script tends to produce lots of noise for things that
can't be fixed in tests like lines in excess of 80 characters
caused by dg-error directives, I tweaked it to suppress these.
To verify that the new checkers don't add too much noise I ran
the patched script on the last 1000 commits, both with and without
testsuite changes (all without libstdc++). The results are below.
It's telling that over a quarter of all commits violate even the
limited subset of the GNU coding guidelines the script checks for.
script w/o tests w/tests
baseline 258 434
patched 282 466
FWIW, to make the script more usable (e.g., to check the rules
appropriate for each file type, including documentation), it
will need to be considerably enhanced. I think most of it can
be done in AWK (already used by the script), and I might try
to tackle that at some point in the future.
Martin
check_GNU_style.sh.diff
contrib/ChangeLog:
2016-02-25 Martin Sebor<mse...@redhat.com>
* check_GNU_style.sh: Add a new global variable.
Add checks for trailing operators and spaces before left brackets.
Tightened up a check for a trailing left curly brace.
(g, ag, vg): Use color.
(col): Don't complain about excessively long lines with DejaGnu
directives.
OK.
FWIW, I think a reasonable long term course would be to continue to
update contrib/clang-format. Ideally we could get to a point where we
could declare the output of clang-format (with the configuration we've
chosen) as the canonical formatting. Which would go a long way towards
having patch submissions & review focus much more on the content rather
than the format of patches.
jeff
