llvm dev - Jan 2020 - merge_guards_bot reports clang-tidy/clang-format findings unrelated to modified code

First, I love this bot:) It makes LLVM's "pushing to master"
practise less awful:)
Now the main topic...
> clang-tidy: fail. Please fix clang-tidy findings.
For example, on https://reviews.llvm.org/D72103#1801916 ,
merge_guards_bot reports clang-tidy findings of existing code, not just
the modified code.

I think https://github.com/google/llvm-premerge-checks should use
--line-filter= as clang-tidy/tool/clang-tidy-diff.py does.
> clang-format: fail. Please format your changes with clang-format by running
git-clang-format HEAD^ or applying this patch.
Similarly, clang-format should use --lines to filter out untouched code.

I second Fangrui's findings and add:

- clang-format should only run on "source" files. For one, it runs on
  non-C/C++ files right now with the results you can imagine. Even if it
  is a C/C++ test we do not always want to format it.
- it would be perfect if the linter would not report issues that are
  present in the surrounding code, e.g., if the naming scheme is kept
  consistent with the surrounding code it should not report it as
  problematic. For example, since the functions in ADT/SetOperations.h
  are all flagged as problematic which means if you add one in the same
  style it is flagged as well.


On 01/02, Fangrui Song via llvm-dev wrote:
> First, I love this bot:) It makes LLVM's "pushing to master"
practise less awful:)
> Now the main topic...
> 
> > clang-tidy: fail. Please fix clang-tidy findings.
> 
> For example, on https://reviews.llvm.org/D72103#1801916 ,
> merge_guards_bot reports clang-tidy findings of existing code, not just
> the modified code.
> 
> I think https://github.com/google/llvm-premerge-checks should use
> --line-filter= as clang-tidy/tool/clang-tidy-diff.py does.
> 
> > clang-format: fail. Please format your changes with clang-format by
running git-clang-format HEAD^ or applying this patch.
> 
> Similarly, clang-format should use --lines to filter out untouched code.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-- 

Johannes Doerfert
Researcher

Argonne National Laboratory
Lemont, IL 60439, USA

jdoerfert at anl.gov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200103/00fddb9b/attachment.sig>

On Fri, Jan 03, 2020 at 04:36:25PM +0000, Doerfert, Johannes via llvm-dev
wrote:
> I second Fangrui's findings and add:
> 
> - clang-format should only run on "source" files. For one, it
runs on
>   non-C/C++ files right now with the results you can imagine. Even if it
>   is a C/C++ test we do not always want to format it.
I already file a bug report to request not to format the tests [1].

[1] https://github.com/google/llvm-premerge-checks/issues/89


Kind regards,
Mark de Wever