For a few weeks now, we’ve had coding style and thinko checks running in CI for GLib, long enough to see that it’s working OK (check out this pipeline!) and is perhaps time to share with others.
There are two jobs in the checks, both of which run in a new
style-check stage of our CI pipeline, which runs before anything else. One job checks the coding style of a merge request, using
clang-format. The other job checks for any lines which introduce
TODO comments (or similar). These jobs are intended to be fast, but also to not fail the pipeline if they produce warnings. Coding style is subjective, and nobody has yet come up with a mechanical style description which doesn’t have ugly corner cases. So the intention of these jobs is to help remind people of the coding style, to avoid reviewers having to check coding style manually, and hence to only end up thinking about it or discussing it when the CI says the style is wrong.
The first job,
style-check-diff, uses a script to work out the diff from the merge request’s branch point, and then passes that to
clang-format-diff.py, a script from LLVM which uses
clang-format to reformat only the changed lines in a diff. If any reformatting occurs, that means the merge request differs from the GLib coding style (as described by
.clang-format) and should be warned about.
There are some limitations to
clang-format: it can’t completely describe how the GLib coding style aligns function arguments. That’s unfortunate, because GNOME Builder aligns function arguments by default (and it’s nice behaviour). Hopefully somebody will implement support in
clang-format for this sometime, so that it can accurately describe the coding style which is used by a large number of GNOME projects.
These coding style checks are work by Pavlo Solntsev and Emmanuel Fleury. Thanks!
The second job,
check-todos, also uses a script to work out the diff from the merge request’s branch point. It passes that to a little Python program which checks for banned words in the commit message and added lines in the diff. The aim is to prevent any comments containing
WIP from accidentally getting merged, as these strings are often used by developers to indicate something they need to come back to before they’re finished — and it’s easy to miss them!
I’ve been using the convention that comments containing
FIXME are OK to be merged, as they indicate something that will need updating in future, but can be merged as-is for now (such as a workaround).
Feedback, improvements, and copying welcome. These scripts should run on any CI image which has
clang and Python.