Re: [PATCH] perf: pass STAGING_INCDIR(sysroot) to perf

[Darren Hart <darren.hart@intel.com>]

On 08/08/2012 06:52 PM, Liang Li wrote:
> On 2012-08-09 09:41, Darren Hart <darren.hart@intel.com> wrote:
>> On 08/08/2012 06:24 PM, Liang Li wrote:
>>> On 2012-08-09 08:36, Darren Hart <darren.hart@intel.com> wrote:
>>>> On 08/07/2012 08:37 PM, Liang Li wrote:
>>>>> On 2012-08-07 22:02, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
>>>>>> On Fri, 2012-08-03 at 23:43 +0800, Liang Li wrote:
>>>>>>> Via EXTRA_CFLAGS, we can pass the sysroot include directory to perf to
>>>>>>> provide slang.h rather than hardcoded host dir in perf's Makefile.
>>>>>>>
>>>>>>> Pass WERROR=0 to perf's Makefile to avoid warnings being treated
>>>>>>> as errors. Warnings are not fatal, and while they will be fixed in the
>>>>>>> future, there's no need for them to break the build.
>>>>>>
>>>>>> No mention of the additional slang dependency is made here?
>>>>>>
>>>>>
>>>>> Forgot mentioned it. Good catch, but the one line change that add
>>>>> slang to DEPENDS seems clear enough for everyone, isn't? :)
>>>>
>>>> Nope, the patch header declares the intent of the patch.
>>>>
>>>
>>> Can't agree with you anymore. :) But we don't list all changes(even
>>> one line straight forward change) in patch header, don't we? :)
>>
>> OK, so we're getting off into the weeds here for a relatively minor
>> change. However, the answer to your question is YES we do list all the
>> changes in the patch header. We don't list them individually, but rather
>> group them functionally.
> 
> Still agree. Yes, we did this all along the time, no confusion here.
> Then you think the one line change in this patch, to add slang to
> DEPENDS, is functional, *should* be listed out in patch header
> explicitly. That is our deviation here, I thought that is
> straightforward, and the patch is *mainly* about *fix* an issue other
> than add a function, the one line change is just part of fix so I was
> mainly focus on explain intention of the fix. :)
> 
>> Adding a dependency to a recipe is a very clear
>> example of a functional change that should be made explicit in the
>> change log in my opinion.
>>
> 
> No problem, add it to patch header would be fine to me, but here let's
> focusing on 'do we need changes in this patch'.

As for me, I am happy with the patch content itself.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel