Re: [Xen-devel] [PATCH] create-diff-object: more precisely identify .rodata sections

From: Julien Grall <julien.grall@arm.com>
To: Lars Kurth <lars.kurth@citrix.com>,
	"Wieczorkiewicz, Pawel" <wipawel@amazon.de>,
	Ian Jackson <Ian.Jackson@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	KonradRzeszutek Wilk <konrad.wilk@oracle.com>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>,
	"Tim \(Xen.org\)" <tim@xen.org>,
	George Dunlap <George.Dunlap@citrix.com>,
	"Pohlack, Martin" <mpohlack@amazon.de>,
	Jan Beulich <jbeulich@suse.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH] create-diff-object: more precisely identify .rodata sections
Date: Thu, 19 Sep 2019 10:22:09 +0100	[thread overview]
Message-ID: <dd7dd06c-0241-991e-7e94-ee465d7134ca@arm.com> (raw)
In-Reply-To: <DE970A9E-2639-4903-98C0-D61D1186B584@citrix.com>

Hi Lars,

On 18/09/2019 12:50, Lars Kurth wrote:
> 
> 
> On 18/09/2019, 11:44, "Wieczorkiewicz, Pawel" <wipawel@amazon.de> wrote:
> 
>      > On 18. Sep 2019, at 12:41, Ian Jackson <ian.jackson@citrix.com> wrote:
>      >
>      > Julien Grall writes ("Re: [PATCH] create-diff-object: more precisely identify .rodata sections"):
>      >> On 18/09/2019 10:52, Wieczorkiewicz, Pawel wrote:
>      >>> $ scripts/./add_maintainers.pl -d ~/git/livepatch-build-tools
>      >>
>      >> '-d' only tells you where the patches files are. The script will look up for the
>      >> MAINTAINERS file in the current directory.
>      >
>      > Hmmm.  I wonder if we could detect this situation somehow.  This will
>      > be a common user error I think.
> 
> I don't think it is possible to detect that situation as git format-patch does not tell you which tree a patch was generated from.
>      
>      I should have looked twice before sending the patch out. But, what would be very helpful for me
>      is additional option to the add_maintainers.pl script like: -m ./MAINTAINERS
>      
> In my view this is only really an issue if you create a patch or series and then do something else before finalizing and sending the patch, otherwise I would have tripped over this myself. But of course, if you work on multiple series at the same time that is an easy mistake to make.
> 
> I would expect that the most common directory structure for people is to have a directory structure such as
> ~/code/xen.git
> ~/code/livepatch-build-tools
> ...
> ~/code/patches
> 
> and that people switch between git directories. Looking at the code, I should be able to add a -m option, which would work out the directory in which MAINTAINERS is, then switch to it, do the processing and switch back to where we started from.
> 
> However, this would only really work, if there was a strong recommendation in https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Using_add_maintainers.pl_.28or_get_maintainer.pl.29_from_outside_of_xen.git telling people to use -m $path/MAKEFILE when working on multiple directories

I don't really see any advantage of this option if you still allow as a fallback 
to run on the current directory.

Ok, the user is saving 2 instructions, but there are still way for that users to 
mess it up such as it may forget the -m option because it misread the wiki.

So I would strongly suggest to either drop the fallback or not adding a new option.

Furthermore, if you let the user specific the existing MAINTAINERS file, then 
he/she might as well pass something different (like MAKEFILE in your example 
;)). It would be best if the users is not allow to give anything else than 
MAINTAINERS.

Cheers,

-- 
Julien Grall

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