* [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
@ 2014-11-18 20:03 Julien Grall
2014-11-20 16:08 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Julien Grall @ 2014-11-18 20:03 UTC (permalink / raw)
To: xen-devel; +Cc: Julien Grall, ian.jackson, ian.campbell, Don Slutz
By default, the script get_maintainer.pl will remove duplicates email as soon
as it appends the list of maintainers of a new file, and therefore override
the role of the developper.
On complex patch (see [1]), this will result to ommitting randomly some
maintainers.
This could be fixed by not removing the duplicate email in the list. Once the
list is created, when it's necessary, the script will drop the "REST" people
and remove duplicata.
Example:
Patch: https://patches.linaro.org/41083/
Before:
Daniel De Graaf <dgdegra@tycho.nsa.gov>
Ian Jackson <ian.jackson@eu.citrix.com>
Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Ian Campbell <ian.campbell@citrix.com>
Wei Liu <wei.liu2@citrix.com>
George Dunlap <george.dunlap@eu.citrix.com>
xen-devel@lists.xen.org
After:
Daniel De Graaf <dgdegra@tycho.nsa.gov>
Ian Jackson <ian.jackson@eu.citrix.com>
Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Ian Campbell <ian.campbell@citrix.com>
Wei Liu <wei.liu2@citrix.com>
Stefano Stabellini <stefano.stabellini@citrix.com>
Tim Deegan <tim@xen.org>
Keir Fraser <keir@xen.org>
Jan Beulich <jbeulich@suse.com>
George Dunlap <george.dunlap@eu.citrix.com>
xen-devel@lists.xen.org
[1] http://lists.xenproject.org/archives/html/xen-devel/2014-11/msg00060.html
Signed-off-by: Julien Grall <julien.grall@linaro.org>
CC: Don Slutz <dslutz@verizon.com>
---
I would like to see this patch in Xen 4.5 and backported to Xen 4.4 (first
time the script has been introduced).
Developpers using this script won't ommitted to cc some maintainers, and it
will avoid maintainers complaining about miss CC.
The only drawbacks I can see is there is too much people CCed (the
patch d67738db was intended to avoid CCing Keir too often).
Also, if the maintainers is referenced twice in the file MAINTAINERS with
different email, the script won't notice it's duplicated and list 2 times.
Though, for this one it could be fixed by modifying the MAINTAINERS file.
Is it worth for Xen 4.5? For know, it seems to only happen with Stefano.
---
scripts/get_maintainer.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index df920e2..cc445cd 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -35,7 +35,7 @@ my $email_git_min_percent = 5;
my $email_git_since = "1-year-ago";
my $email_hg_since = "-365";
my $interactive = 0;
-my $email_remove_duplicates = 1;
+my $email_remove_duplicates = 0;
my $email_use_mailmap = 1;
my $email_drop_the_rest_supporter_if_supporter_found = 1;
my $output_multiline = 1;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-18 20:03 [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers Julien Grall
@ 2014-11-20 16:08 ` Ian Campbell
2014-11-20 16:15 ` Julien Grall
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2014-11-20 16:08 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, Don Slutz
On Tue, 2014-11-18 at 20:03 +0000, Julien Grall wrote:
> By default, the script get_maintainer.pl will remove duplicates email as soon
> as it appends the list of maintainers of a new file, and therefore override
> the role of the developper.
>
> On complex patch (see [1]), this will result to ommitting randomly some
> maintainers.
>
> This could be fixed
Are you proposing an alternative/better fix here? or describing what
this patch does?
> by not removing the duplicate email in the list. Once the
> list is created, when it's necessary, the script will drop the "REST" people
> and remove duplicata.
>
> Example:
>
> Patch: https://patches.linaro.org/41083/
>
> Before:
>
> Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Ian Jackson <ian.jackson@eu.citrix.com>
> Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Ian Campbell <ian.campbell@citrix.com>
> Wei Liu <wei.liu2@citrix.com>
> George Dunlap <george.dunlap@eu.citrix.com>
> xen-devel@lists.xen.org
>
> After:
>
> Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Ian Jackson <ian.jackson@eu.citrix.com>
> Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Ian Campbell <ian.campbell@citrix.com>
> Wei Liu <wei.liu2@citrix.com>
> Stefano Stabellini <stefano.stabellini@citrix.com>
> Tim Deegan <tim@xen.org>
> Keir Fraser <keir@xen.org>
> Jan Beulich <jbeulich@suse.com>
> George Dunlap <george.dunlap@eu.citrix.com>
> xen-devel@lists.xen.org
>
> [1] http://lists.xenproject.org/archives/html/xen-devel/2014-11/msg00060.html
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> CC: Don Slutz <dslutz@verizon.com>
>
> ---
> I would like to see this patch in Xen 4.5 and backported to Xen 4.4 (first
> time the script has been introduced).
>
> Developpers using this script won't ommitted to cc some maintainers, and it
> will avoid maintainers complaining about miss CC.
>
> The only drawbacks I can see is there is too much people CCed (the
> patch d67738db was intended to avoid CCing Keir too often).
My tree doesn't have in it d67738db but from the example you give above
it seems like this patch will regress that? As someone who already gets
too much mail and is listed in THE REST these days I am very much in
favour of not mailing THE REST when other maintainers have been found.
> Also, if the maintainers is referenced twice in the file MAINTAINERS with
> different email, the script won't notice it's duplicated and list 2 times.
> Though, for this one it could be fixed by modifying the MAINTAINERS file.
> Is it worth for Xen 4.5? For know, it seems to only happen with Stefano.
That's fine IMHO. The script shouldn't be expected to be smart enough to
reconcile two distinct strings which happen to refer to the same person
into a single string. If someone cares they should patch MAINTAINERS to
refer to themselves in a consistent way.
> ---
> scripts/get_maintainer.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index df920e2..cc445cd 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -35,7 +35,7 @@ my $email_git_min_percent = 5;
> my $email_git_since = "1-year-ago";
> my $email_hg_since = "-365";
> my $interactive = 0;
> -my $email_remove_duplicates = 1;
> +my $email_remove_duplicates = 0;
> my $email_use_mailmap = 1;
> my $email_drop_the_rest_supporter_if_supporter_found = 1;
> my $output_multiline = 1;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:08 ` Ian Campbell
@ 2014-11-20 16:15 ` Julien Grall
2014-11-20 16:21 ` Julien Grall
2014-11-20 16:30 ` Ian Campbell
0 siblings, 2 replies; 11+ messages in thread
From: Julien Grall @ 2014-11-20 16:15 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, ian.jackson, Don Slutz
Hi Ian,
On 11/20/2014 04:08 PM, Ian Campbell wrote:
> On Tue, 2014-11-18 at 20:03 +0000, Julien Grall wrote:
>> By default, the script get_maintainer.pl will remove duplicates email as soon
>> as it appends the list of maintainers of a new file, and therefore override
>> the role of the developper.
>>
>> On complex patch (see [1]), this will result to ommitting randomly some
>> maintainers.
>>
>> This could be fixed
>
> Are you proposing an alternative/better fix here? or describing what
> this patch does?
Describing what the patch does.
>> by not removing the duplicate email in the list. Once the
>> list is created, when it's necessary, the script will drop the "REST" people
>> and remove duplicata.
>>
>> Example:
>>
>> Patch: https://patches.linaro.org/41083/
>>
>> Before:
>>
>> Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> Ian Jackson <ian.jackson@eu.citrix.com>
>> Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Ian Campbell <ian.campbell@citrix.com>
>> Wei Liu <wei.liu2@citrix.com>
>> George Dunlap <george.dunlap@eu.citrix.com>
>> xen-devel@lists.xen.org
>>
>> After:
>>
>> Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> Ian Jackson <ian.jackson@eu.citrix.com>
>> Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Ian Campbell <ian.campbell@citrix.com>
>> Wei Liu <wei.liu2@citrix.com>
>> Stefano Stabellini <stefano.stabellini@citrix.com>
>> Tim Deegan <tim@xen.org>
>> Keir Fraser <keir@xen.org>
>> Jan Beulich <jbeulich@suse.com>
>> George Dunlap <george.dunlap@eu.citrix.com>
>> xen-devel@lists.xen.org
>>
>> [1] http://lists.xenproject.org/archives/html/xen-devel/2014-11/msg00060.html
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> CC: Don Slutz <dslutz@verizon.com>
>>
>> ---
>> I would like to see this patch in Xen 4.5 and backported to Xen 4.4 (first
>> time the script has been introduced).
>>
>> Developpers using this script won't ommitted to cc some maintainers, and it
>> will avoid maintainers complaining about miss CC.
>>
>> The only drawbacks I can see is there is too much people CCed (the
>> patch d67738db was intended to avoid CCing Keir too often).
>
> My tree doesn't have in it d67738db but from the example you give above
> it seems like this patch will regress that? As someone who already gets
> too much mail and is listed in THE REST these days I am very much in
> favour of not mailing THE REST when other maintainers have been found.
It's still the case with this patch. Before if a maintainer was both in
x86 section and "THE REST". It may end up to completely drop the
maintainer in the CC list.
By drawbacks I meant, if there is another bug in the script then we may
end up to cc too many people. Honestly I don't believe it's the case.
>> Also, if the maintainers is referenced twice in the file MAINTAINERS with
>> different email, the script won't notice it's duplicated and list 2 times.
>> Though, for this one it could be fixed by modifying the MAINTAINERS file.
>> Is it worth for Xen 4.5? For know, it seems to only happen with Stefano.
>
Regards.
--
Julien Grall
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:15 ` Julien Grall
@ 2014-11-20 16:21 ` Julien Grall
2014-11-20 16:29 ` Ian Campbell
2014-11-20 16:30 ` Ian Campbell
1 sibling, 1 reply; 11+ messages in thread
From: Julien Grall @ 2014-11-20 16:21 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, ian.jackson, Don Slutz
On 11/20/2014 04:15 PM, Julien Grall wrote:
> Hi Ian,
>
> On 11/20/2014 04:08 PM, Ian Campbell wrote:
>> On Tue, 2014-11-18 at 20:03 +0000, Julien Grall wrote:
>>> By default, the script get_maintainer.pl will remove duplicates email as soon
>>> as it appends the list of maintainers of a new file, and therefore override
>>> the role of the developper.
>>>
>>> On complex patch (see [1]), this will result to ommitting randomly some
>>> maintainers.
>>>
>>> This could be fixed
>>
>> Are you proposing an alternative/better fix here? or describing what
>> this patch does?
>
> Describing what the patch does.
>
>>> by not removing the duplicate email in the list. Once the
>>> list is created, when it's necessary, the script will drop the "REST" people
>>> and remove duplicata.
>>>
>>> Example:
>>>
>>> Patch: https://patches.linaro.org/41083/
>>>
>>> Before:
>>>
>>> Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>> Ian Jackson <ian.jackson@eu.citrix.com>
>>> Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Ian Campbell <ian.campbell@citrix.com>
>>> Wei Liu <wei.liu2@citrix.com>
>>> George Dunlap <george.dunlap@eu.citrix.com>
>>> xen-devel@lists.xen.org
>>>
>>> After:
>>>
>>> Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>> Ian Jackson <ian.jackson@eu.citrix.com>
>>> Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Ian Campbell <ian.campbell@citrix.com>
>>> Wei Liu <wei.liu2@citrix.com>
>>> Stefano Stabellini <stefano.stabellini@citrix.com>
>>> Tim Deegan <tim@xen.org>
>>> Keir Fraser <keir@xen.org>
>>> Jan Beulich <jbeulich@suse.com>
>>> George Dunlap <george.dunlap@eu.citrix.com>
>>> xen-devel@lists.xen.org
>>>
>>> [1] http://lists.xenproject.org/archives/html/xen-devel/2014-11/msg00060.html
>>>
>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>> CC: Don Slutz <dslutz@verizon.com>
>>>
>>> ---
>>> I would like to see this patch in Xen 4.5 and backported to Xen 4.4 (first
>>> time the script has been introduced).
>>>
>>> Developpers using this script won't ommitted to cc some maintainers, and it
>>> will avoid maintainers complaining about miss CC.
>>>
>>> The only drawbacks I can see is there is too much people CCed (the
>>> patch d67738db was intended to avoid CCing Keir too often).
>>
>> My tree doesn't have in it d67738db but from the example you give above
>> it seems like this patch will regress that? As someone who already gets
>> too much mail and is listed in THE REST these days I am very much in
>> favour of not mailing THE REST when other maintainers have been found.
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.
--
Julien Grall
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:21 ` Julien Grall
@ 2014-11-20 16:29 ` Ian Campbell
2014-11-20 16:43 ` Julien Grall
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2014-11-20 16:29 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, Don Slutz
On Thu, 2014-11-20 at 16:21 +0000, Julien Grall wrote:
> On 11/20/2014 04:15 PM, Julien Grall wrote:
> > Hi Ian,
> >
> > On 11/20/2014 04:08 PM, Ian Campbell wrote:
> >> On Tue, 2014-11-18 at 20:03 +0000, Julien Grall wrote:
> >>> By default, the script get_maintainer.pl will remove duplicates email as soon
> >>> as it appends the list of maintainers of a new file, and therefore override
> >>> the role of the developper.
> >>>
> >>> On complex patch (see [1]), this will result to ommitting randomly some
> >>> maintainers.
> >>>
> >>> This could be fixed
> >>
> >> Are you proposing an alternative/better fix here? or describing what
> >> this patch does?
> >
> > Describing what the patch does.
> >
> >>> by not removing the duplicate email in the list. Once the
> >>> list is created, when it's necessary, the script will drop the "REST" people
> >>> and remove duplicata.
> >>>
> >>> Example:
> >>>
> >>> Patch: https://patches.linaro.org/41083/
> >>>
> >>> Before:
> >>>
> >>> Daniel De Graaf <dgdegra@tycho.nsa.gov>
> >>> Ian Jackson <ian.jackson@eu.citrix.com>
> >>> Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>> Ian Campbell <ian.campbell@citrix.com>
> >>> Wei Liu <wei.liu2@citrix.com>
> >>> George Dunlap <george.dunlap@eu.citrix.com>
> >>> xen-devel@lists.xen.org
> >>>
> >>> After:
> >>>
> >>> Daniel De Graaf <dgdegra@tycho.nsa.gov>
> >>> Ian Jackson <ian.jackson@eu.citrix.com>
> >>> Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>> Ian Campbell <ian.campbell@citrix.com>
> >>> Wei Liu <wei.liu2@citrix.com>
> >>> Stefano Stabellini <stefano.stabellini@citrix.com>
> >>> Tim Deegan <tim@xen.org>
> >>> Keir Fraser <keir@xen.org>
> >>> Jan Beulich <jbeulich@suse.com>
> >>> George Dunlap <george.dunlap@eu.citrix.com>
> >>> xen-devel@lists.xen.org
> >>>
> >>> [1] http://lists.xenproject.org/archives/html/xen-devel/2014-11/msg00060.html
> >>>
> >>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >>> CC: Don Slutz <dslutz@verizon.com>
> >>>
> >>> ---
> >>> I would like to see this patch in Xen 4.5 and backported to Xen 4.4 (first
> >>> time the script has been introduced).
> >>>
> >>> Developpers using this script won't ommitted to cc some maintainers, and it
> >>> will avoid maintainers complaining about miss CC.
> >>>
> >>> The only drawbacks I can see is there is too much people CCed (the
> >>> patch d67738db was intended to avoid CCing Keir too often).
> >>
> >> My tree doesn't have in it d67738db but from the example you give above
> >> it seems like this patch will regress that? As someone who already gets
> >> too much mail and is listed in THE REST these days I am very much in
> >> favour of not mailing THE REST when other maintainers have been found.
>
> 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?
Ian.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:29 ` Ian Campbell
@ 2014-11-20 16:43 ` Julien Grall
2014-11-20 16:52 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Julien Grall @ 2014-11-20 16:43 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, ian.jackson, Don Slutz
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?
Yes, my description was confusing. With setting $email_remove_duplicates
to 0, the script will:
1) Append the list of maintainers for every file
2) Filter the list to remove the entry with "THE REST" role
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
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.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:43 ` Julien Grall
@ 2014-11-20 16:52 ` Ian Campbell
2014-11-20 16:57 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2014-11-20 16:52 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, Don Slutz
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?
>
> Yes, my description was confusing. With setting $email_remove_duplicates
> to 0, the script will:
> 1) Append the list of maintainers for every file
> 2) Filter the list to remove the entry with "THE REST" role
> 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
>
> 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.
Ian.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:52 ` Ian Campbell
@ 2014-11-20 16:57 ` Ian Campbell
2014-11-20 17:20 ` Julien Grall
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2014-11-20 16:57 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, Don Slutz
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?
> > 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?
> > 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?
> > 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.
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:57 ` Ian Campbell
@ 2014-11-20 17:20 ` Julien Grall
0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2014-11-20 17:20 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, ian.jackson, Don Slutz
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:15 ` Julien Grall
2014-11-20 16:21 ` Julien Grall
@ 2014-11-20 16:30 ` Ian Campbell
2014-11-20 17:20 ` Julien Grall
1 sibling, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2014-11-20 16:30 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ian.jackson, Don Slutz
On Thu, 2014-11-20 at 16:15 +0000, Julien Grall wrote:
> Hi Ian,
>
> On 11/20/2014 04:08 PM, Ian Campbell wrote:
> > On Tue, 2014-11-18 at 20:03 +0000, Julien Grall wrote:
> >> By default, the script get_maintainer.pl will remove duplicates email as soon
> >> as it appends the list of maintainers of a new file, and therefore override
> >> the role of the developper.
> >>
> >> On complex patch (see [1]), this will result to ommitting randomly some
> >> maintainers.
> >>
> >> This could be fixed
> >
> > Are you proposing an alternative/better fix here? or describing what
> > this patch does?
>
> Describing what the patch does.
Then I think you meant "This is fixed" or "This patches fixes this
by ...".
> By drawbacks I meant, if there is another bug in the script then we may
> end up to cc too many people. Honestly I don't believe it's the case.
Sure, lets assume not.
Ian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers
2014-11-20 16:30 ` Ian Campbell
@ 2014-11-20 17:20 ` Julien Grall
0 siblings, 0 replies; 11+ messages in thread
From: Julien Grall @ 2014-11-20 17:20 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, ian.jackson, Don Slutz
On 11/20/2014 04:30 PM, Ian Campbell wrote:
> On Thu, 2014-11-20 at 16:15 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 11/20/2014 04:08 PM, Ian Campbell wrote:
>>> On Tue, 2014-11-18 at 20:03 +0000, Julien Grall wrote:
>>>> By default, the script get_maintainer.pl will remove duplicates email as soon
>>>> as it appends the list of maintainers of a new file, and therefore override
>>>> the role of the developper.
>>>>
>>>> On complex patch (see [1]), this will result to ommitting randomly some
>>>> maintainers.
>>>>
>>>> This could be fixed
>>>
>>> Are you proposing an alternative/better fix here? or describing what
>>> this patch does?
>>
>> Describing what the patch does.
>
> Then I think you meant "This is fixed" or "This patches fixes this
> by ...".
Ok. I will fix it.
>
>> By drawbacks I meant, if there is another bug in the script then we may
>> end up to cc too many people. Honestly I don't believe it's the case.
>
> Sure, lets assume not.
I will drop this drawbacks.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-11-20 17:20 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-18 20:03 [PATCH for-4.5] scripts/get_maintainer.pl: Correctly CC the maintainers Julien Grall
2014-11-20 16:08 ` Ian Campbell
2014-11-20 16:15 ` Julien Grall
2014-11-20 16:21 ` Julien Grall
2014-11-20 16:29 ` Ian Campbell
2014-11-20 16:43 ` Julien Grall
2014-11-20 16:52 ` Ian Campbell
2014-11-20 16:57 ` Ian Campbell
2014-11-20 17:20 ` Julien Grall
2014-11-20 16:30 ` Ian Campbell
2014-11-20 17:20 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.