[cfe-dev] [llvm-dev] Modernizing LLVM Coding Style Guide and enforcing Clang-tidy

Renato Golin via cfe-dev cfe-dev at lists.llvm.org
Tue Jan 10 03:17:23 PST 2017


On 10 January 2017 at 08:37, Mehdi Amini <mehdi.amini at apple.com> wrote:
> Well I believe it still buys you an interesting property: no bike shedding
> over “personal preference” in any review. There’s a guideline to point at
> and we can’t instead focus on the important bits.

You can't have that certainty with guidelines.

These 4 examples are less enforceable than 80-columns or
no-tabs-2-spaces rules, despite both sets being personal. They're
clearly guidelines.

All writing it down will buy you is "prefer this, not that", but:

1. If you have a good reason to do that, please do.
2. If the surrounding code uses that pattern, and this is not a
structural change, please repeat.
3. If it makes no difference as to the clarity of the code, who cares?

The most important thing here is the reaction of the community.

Reverting patches because they don't conform to guidelines is a big
mistake. Even 80 columns violation should be fixed later, in place, by
the original committer, after post-commit review.

Blocking patches because they don't conform to personal preferences,
even if they're encoded in the coding standard as "prefer" or
"should", ignoring the other facts I mention above, is also really bad
form.

First, we don't have code stewards to clean up other people's mistakes
or preferences, not we should. This would create a culture of garbage
code, where people don't care about what they commit.

Second, we want developers to feel comfortable contributing to our
project. People DO have different cultures, preferences, levels of
comfort with the languages and the project. But they also have their
own lives and work, and can't spend months polishing small patches,
just because they're not perfect in the view of random developers in
our community. That would be extremely counter-productive and would
ultimately kill the project.

Similar behaviour has killed other OSS projects in the past, so this
is not theoretical.

All that said, I think those 4 *guidelines* are nice to have on our
coding standards. But I'm ultimately against forcing people to run
clang-tidy or blocking/reverting patches because they don't match
perfectly with the coding standards.

cheers,
--renato



More information about the cfe-dev mailing list