aaron.ballman added a comment.

In D112881#3160590 <https://reviews.llvm.org/D112881#3160590>, @paulaltin wrote:

>> LGTM, with a few small nits. It'd be nice to update the patch summary with 
>> more information about why the option is needed.
>
> Thanks @aaron.ballman. I've made the changes to the Release Notes and added 
> more info to the summary.

Thanks! Do you need someone to commit on your behalf? If so, what name and 
email address would you like used for patch attribution?

> In general, would you be happy to consider other changes like this in 
> clang-tidy? Our company uses the tool as part of our CI (and I personally 
> find it extremely useful), but there are some checks that we have to disable 
> entirely because they do multiple things, some of which we don't want to (or 
> can't easily) fix. If these had options to disable part of the check then we 
> would still be able to use the remaining parts, which would be great.
>
> If adding more options to existing checks in clang-tidy is a direction that 
> the developers are happy to support, then I'd be keen to submit more changes 
> like this.

We're definitely happy to consider these kinds of changes, thank you! We 
usually prefer finding ways to silence the diagnostic in code (like casting to 
void to silence an unused variable warning, etc) over configuration options, 
but that's not always plausible to do and so extra configuration options are a 
good fallback solution.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112881/new/

https://reviews.llvm.org/D112881

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to