From: Lars Kurth <lars.kurth@citrix.com>
To: Julien Grall <julien.grall@arm.com>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Andrew Cooper <Andrew.Cooper3@citrix.com>,
"Tim \(Xen.org\)" <tim@xen.org>,
George Dunlap <George.Dunlap@citrix.com>,
Jan Beulich <jbeulich@suse.com>,
Ian Jackson <Ian.Jackson@citrix.com>
Subject: Re: [Xen-devel] [PATCH v2] Allow get_maintainer.pl / add_maintainers.pl scripts to be called outside of xen.git
Date: Fri, 16 Aug 2019 12:17:22 +0000 [thread overview]
Message-ID: <E16EC776-8DAC-439A-9FF9-E5DF1F18206B@citrix.com> (raw)
In-Reply-To: <348ae310-a048-0e96-dc12-981f96450a5f@arm.com>
On 16/08/2019, 11:01, "Julien Grall" <julien.grall@arm.com> wrote:
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.
I thought about this and it is not as easy as it seems, because the script only parses
M: ... &c lines
Maybe the best way to address this would be to include some identifier into the
MAINTAINERS file (after the header with all the definitions).
FORMAT: xen-project-maintainers <version>
(note that this is not currently picked up by the tool)
Or
V: xen-project-maintainers <version>
(note that this would be picked up by the tool)
Then any compliant version is easily identifiable and the warning can be supressed.
That would make maintainers look like
============
....
V: xen-project-maintainers 1.6
ACPI
M: Jan Beulich <jbeulich@suse.com>
...
============
This seems to be more of a can of worms than I thought.
Cheers
Lars
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-08-16 12:17 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-15 17:22 [Xen-devel] [PATCH v2] Allow get_maintainer.pl / add_maintainers.pl scripts to be called outside of xen.git Lars Kurth
2019-08-16 10:00 ` Julien Grall
2019-08-16 12:17 ` Lars Kurth [this message]
2019-08-16 13:28 ` Julien Grall
2019-08-16 13:43 ` Lars Kurth
2019-08-23 16:34 ` Lars Kurth
2019-08-24 1:16 ` Stefano Stabellini
2019-08-27 17:13 ` Ian Jackson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E16EC776-8DAC-439A-9FF9-E5DF1F18206B@citrix.com \
--to=lars.kurth@citrix.com \
--cc=Andrew.Cooper3@citrix.com \
--cc=George.Dunlap@citrix.com \
--cc=Ian.Jackson@citrix.com \
--cc=jbeulich@suse.com \
--cc=julien.grall@arm.com \
--cc=konrad.wilk@oracle.com \
--cc=sstabellini@kernel.org \
--cc=tim@xen.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).