Re: [Xen-devel] [PATCH v2] Allow get_maintainer.pl / add_maintainers.pl scripts to be called outside of xen.git

[Julien Grall <julien.grall@arm.com>]

Hi Lars,

Thank you for the patch.

On 15/08/2019 18:22, Lars Kurth wrote:
> Use-case: Allow using both scripts on xen repositories such as
> mini-os.git, osstest.git,
> 
> Tool changes:
> * add_maintainers.pl: $get_maintainer inherits path from $0
> * get_maintainer.pl: warn (instead fo die) when called
>    from a different tree
> 
> Assumptions: the repository contains a MAINTAINERS file that
> follows the same conventions as the file in xen.git
> 
> A suggested template
> 
> ========================================================
> This file follows the same conventions as outlined in
> xen.git:MAINTAINERS. Please refer to the file in xen.git
> for more information.
> 
> THE REST
> M:	MAINTAINER1 <maintainer1@email.com>
> M:      MAINTAINER2 <maintainer2@email.com>
> L:	xen-devel@lists.xenproject.org
> S:	Supported
> F:	*/
> ========================================================
> 
> Signed-off-by: Lars Kurth <lars.kurth@citrix.com>
> ---
>   scripts/add_maintainers.pl |  4 ++--
>   scripts/get_maintainer.pl  | 13 +++++++++++--
>   2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/add_maintainers.pl b/scripts/add_maintainers.pl
> index 09e9f6609f..5a6d0f631b 100755
> --- a/scripts/add_maintainers.pl
> +++ b/scripts/add_maintainers.pl
> @@ -26,9 +26,9 @@ sub insert ($$$$);
>   sub hastag ($$);
>   
>   # Tool Variables
> -my $get_maintainer      = "./scripts/get_maintainer.pl";
> -
>   my $tool = $0;
> +my $get_maintainer = $tool;
> +$get_maintainer =~ s/add_maintainers/get_maintainer/;
>   my $usage = <<EOT;
>   OPTIONS:
>   --------
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 174dfb7e40..f1e9c904ee 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -266,8 +266,17 @@ if ($email &&
>   }
>   
>   if (!top_of_tree($xen_path)) {
> -    die "$P: The current directory does not appear to be "
> -	. "a Xen source tree.\n";
> +    # Do not exit, but print an error message to STDERR to allow calling
> +    # the tool from xen-related repos such as mini-os.git,
> +    # live patch-build-tools.git, etc
> +    print STDERR "$P:\n".
> +          "=====================================================\n".
> +          "WARNING: The current directory does not appear to be \n".
> +	  "the xen.git source tree.\n\n".
> +          "The tool works outside of the xen.git tree, if the\n".
> +          "MAINTAINERS file follows the same format as that of\n".
> +          "xen.git. Use at your own peril.\n".
> +          "=====================================================\n";

 From my understanding, any use on mini-os.git & co will be legitimate. However, 
we still print the WARNING in those cases.

Usually WARNING means something needs attention. As most of the users will 
likely copy/paste from the wiki, we are going to have report asking why the 
WARNING is there.

I think it would make sense to try to downgrade the message a bit when possible. 
For instance, we could check if the section "THE REST" is present in the file 
MAINTAINERS. If not, this is likely not a file we are able to support.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel