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

Eugene Zelenko via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 3 15:39:43 PST 2017


On Fri, Mar 3, 2017 at 3:27 PM, Vedant Kumar <vsk at apple.com> wrote:
>
>> On Mar 3, 2017, at 3:12 PM, Eugene Zelenko <eugene.zelenko at gmail.com> wrote:
>>
>> On Fri, Mar 3, 2017 at 3:01 PM, Vedant Kumar <vsk at apple.com> 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.
>>>
>>>  * 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.
>>
>> Changes are small for each file and uniform for entire set.
>
> Due to the breadth of changes, there are several possible merge conflicts.
>
> That's what we got internally after your commit.
>
>
>>> 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
>>
>> Sure, it'll be much better if code authors take proper care about
>> formatting/tidying it. But what to in cases where this was not done
>> for long time?
>
> I would also like all of our code to adhere to the community standards.
>
> Please accept that this will happen slowly, and that this is a shared responsibility.
>
> Aggressively pushing all of the changes yourself is wasting resources and actually creating problems.
>
> vedant

I only wanted to help.

Eugene.


More information about the llvm-commits mailing list