[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