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

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 3 15:01:56 PST 2017


> 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.

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

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.

vedant


More information about the llvm-commits mailing list