[llvm] r296846 - [ProfileData] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 7 11:05:54 PDT 2017


> On Mar 3, 2017, at 3:01 PM, Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 
>> On Mar 3, 2017, at 2:25 PM, Eugene Zelenko via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> 
>> Hi, Vedant !
>> 
>> Mehdi and Zachary suggested awhile ago that such changes are fine with
>> post-commit reviews.
> 
> I disagree. There are costs to doing this this way:
> 
>  * It pollutes the version control history.

How? This is *not* formatting changes. 

> 
>  * Contributors with out-of-tree code have to deal with the churn.

This has never been a criteria, and I strongly oppose to make it a criteria or even start to consider this while I’m making any kind of upstream change.

> I dislike being forced into reacting to formatting/tidying changes, because I don't think the benefits outweigh the costs.
> 
> 
>> Could you please clarify what do you consider as "invasive"? Frankly,
>> I don't see anything "large" in them.
> 
> Your commit changes several hundred lines of code and touches about 20 files. It's strange that you don't think it's large.
> 
> IMO it's acceptable to make NFC cleanups if you're planning on doing work in that area. Otherwise, it's just invasive churn.
> 
> My main complaint is that there is not a net value-add to this commit.


You conflating clang-format massive changes with tidy/cleanup/refactoring. These are totally separate things IMO.

— 
Mehdi





More information about the llvm-commits mailing list