[llvm-dev] clang-tidy makes review a pain

[Aaron Ballman via llvm-dev]

On Mon, Apr 19, 2021 at 5:43 AM Maxim Kazantsev via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> Hello everyone,
>
> I started noticing that lately we’ve improved reporting from clang-tidy, pointing out at various formatting issues. However the more verbose it becomes, the more annoyed I feel about it. For example here:
>
> https://reviews.llvm.org/D100721

FWIW, all of those diagnostics in the review are correct and are items
you should fix. Having an automated tool tell you about them means
that reviewers have less work to do and the code base is more likely
to end up in a more consistent stylistic state. Having that
information earlier rather than later (such as at patch acceptance)
means less work for code reviewers, who are sometimes already pretty
taxed. That's the whole point to having clang-tidy and clang-format
integrated into CI.

> It complains literally about every second line, inserting its comments straight into review. They take as much space as the actual code. Maybe it’s just me, but it’s really hard to me to understand what the patch is actually doing with so many inlined auto-generated comments. Maybe there is a button to hide them somewhere, but I failed to find it.
>
> I understand what was the intention, and clang-tidy is a cool thing in general, but it’s getting too intrusive. Does anyone else have the same problem as I do? If there’s a lot of people whom it annoys, maybe we should think how to make it less invasive. Maybe it should put these comment when the patch gets approved, or something like this.

I've complained about the verbosity of clang-tidy and clang-format
checks in reviews before, but my concerns come from diagnostics like
clang-format not being found on PATH (not something the code author or
the reviewer can do anything about), confusion with Phabricator's
stacked patches (where clang-tidy will complain about unknown or
missing methods that exist in a parent patch but not in the child
patch), or clang-format trying to format things it shouldn't (tablegen
files, test cases, unit tests, etc).

However, as a reviewer, I think the comments in the review you linked
are a demonstration of the functionality working as-designed how I'd
want it to work.

~Aaron

>
>
>
> Thanks,
>
> Max
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev