diff mbox

[Xen-devel,v2,for,4.5] scripts/get_maintainer.pl: Correctly CC the maintainers

Message ID 1416504963-4830-1-git-send-email-julien.grall@linaro.org
State Accepted, archived
Headers show

Commit Message

Julien Grall Nov. 20, 2014, 5:36 p.m. UTC
The current script is setting $email_remove_duplicates to 1 by default, on
complex patch (see [1]), this will result to ommitting randomly some
maintainers.

This is because, the script will:
    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. => The role will be lost
    2) Filter the list to remove the entry with "THE REST" role

So if a maintainers is marked with "THE REST" role on the first file and
actually be an x86 maintainers on the script, the script will only retain
the "THE REST" role. During the filtering step, this maintainers will
therefore be dropped.

This patch fixes this by setting $email_remove_duplicates to 0 by default.
The new behavior of the script will be:
    1) Append the list of maintainers for every file
    2) Filter the list to remove the entry with "THE REST" role
    3) Remove duplicated email address

Example:

Patch: https://patches.linaro.org/41083/

Before the patch:

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 the patch:

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>

---
    Changes in v2:
        - Rework the commit message to explain the problem and the
        solution more clearly

    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 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(-)

Comments

Konrad Rzeszutek Wilk Nov. 20, 2014, 8:23 p.m. UTC | #1
On Thu, Nov 20, 2014 at 05:36:03PM +0000, Julien Grall wrote:
> The current script is setting $email_remove_duplicates to 1 by default, on
> complex patch (see [1]), this will result to ommitting randomly some
> maintainers.

One could see that as feature - the emails about bugs or patches
to review don't land in your inbox! 

> 
> This is because, the script will:
>     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. => The role will be lost
>     2) Filter the list to remove the entry with "THE REST" role
> 
> So if a maintainers is marked with "THE REST" role on the first file and
> actually be an x86 maintainers on the script, the script will only retain
> the "THE REST" role. During the filtering step, this maintainers will
> therefore be dropped.
> 
> This patch fixes this by setting $email_remove_duplicates to 0 by default.
> The new behavior of the script will be:
>     1) Append the list of maintainers for every file
>     2) Filter the list to remove the entry with "THE REST" role
>     3) Remove duplicated email address
> 
> Example:
> 
> Patch: https://patches.linaro.org/41083/
> 
> Before the patch:
> 
> 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 the patch:
> 
> 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>
> 
> ---
>     Changes in v2:
>         - Rework the commit message to explain the problem and the
>         solution more clearly
> 
>     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 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.

I am OK with this going in Xen 4.5.

Thank you.
> ---
>  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
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
diff mbox

Patch

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;