Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers

[Julien Grall <julien.grall@linaro.org>]

On 11/20/2014 04:57 PM, Ian Campbell wrote:
> On Thu, 2014-11-20 at 16:52 +0000, Ian Campbell wrote:
>> On Thu, 2014-11-20 at 16:43 +0000, Julien Grall wrote:
>>> On 11/20/2014 04:29 PM, Ian Campbell wrote:
>>>>> Forgot to add, the example above show the difference without and with
>>>>> the patch. The list is correct because both ARM and x86 maintainers
>>>>> should be CC. Because of this all "THE REST" maintainers are added.
>>>>
>>>> Just to be clear, you mean that everyone under THE REST is added solely
>>>> because they also happen to be maintainers of some other relevant bit of
>>>> code, not that THE REST is explicitly added in this case, right?
>>>
> 
> Just a small clarification...
> 
>>> Yes, my description was confusing. With setting $email_remove_duplicates
>>> to 0, the script will:
>>>    1) Append the list of maintainers for every file
> 
> At this point each maintainer remains associated with the role/reason
> they are added, right?

Yes. Every time the maintainers is listed we add in the list no matters
if the mail already exists. So the maintainers may be listed twice with
different roles (for instance: x86 and "THE REST").

>>>    2) Filter the list to remove the entry with "THE REST" role
> 
> And this only happens if there are roles other than "THE REST" in the
> list?

Yes.

> 
>>>    3) Remove duplicated address
>>>
>>> The previous behavior was:
>>>    1) Get the list of maintainers of the file (incidentally all the
>>> maintainers in "THE REST" role are added). If the email address already
>>> exists in the global list, skip it.
>>>    2) Filter the list to remove the entry with "THE REST" role
> 
> Whereas here the link from maintainer to the role is lost, hence
> everyone in THE REST is blindly removed?

Yes.

>>> So if a maintainers is marked on the "THE REST" on the first file and
>>> actually be an x86 maintainers on the second file, the scripts will only
>>> retain the "THE REST" role.
>>>
>>> If it's more clear, I can add the explanation above in the commit message.
>>
>> It is, please do.

I will send a new version.

Regards,


-- 
Julien Grall