[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On Fri, 23 Aug 2019, Lars Kurth wrote:
> 16/08/2019, 06:43, "Lars Kurth" <lars.kurth.xen@xxxxxxxxx> wrote:
>     > On 16 Aug 2019, at 14:28, Julien Grall <julien.grall@xxxxxxx> wrote:
>     > 
>     > 
>     > 
>     > On 16/08/2019 13:17, Lars Kurth wrote:
>     >> On 16/08/2019, 11:01, "Julien Grall" <julien.grall@xxxxxxx> wrote:
>     >>      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
>     > 
>     > The script is able to parse the section name. See get_maintainer_role().
>     > 
>     > Although, I am not sure how early the function can get called.
>     > 
>     > But...
>     That may make it feasible to go down that route.
>     Incidentially both Linux as well as QEMU MAINTAINERs files use the same 
> syntax
>     as us (with a few extra tags which we don't have)
>     Not sure whether this would be a problem
>     >> 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)
>     > 
>     > Any of these solutions are also a potential alternative.
>     I will see what others think and take it from there
> Hi all. I would like to get this resolved and was looking for 
> opinions. The thread is about enabling usage of get_maintainer.pl / 
> add_maintainers.pl on sister repositories for xen.git, such as 
> xtf.git, osstest.git, mini-os.git, ... to have a consistent tools story 
> and make patch submission for newcomers easier. We have 
> several options:
> 1) Warn if the tools are applied outside the Xen tree
> Julian felt this is likely confusing
> 2) Do not warn under some conditions
> 2.1) Use THE REST as identifier to avoid the warning
> Cons: Warning would disappear because Linux and QEMU also have THE REST 
> This may not be an issue as both MAINTAINERS files follow the same format
> However, there may be subtle differences in behaviour for unusual options 
> for the get_maintainer.pl script as we have not been tracking all changes
> 2.2) Introduce a unique identifier in MAINTAINERS
> This would imply introducing a unique identifier for xen related
> Pros: More accurate
> Cons: Pollutes file format
> I don’t have a strong opinion and will follow majority consensus.
> Maybe people can vote on the options and I will just implement
> what most people prefer
Any of these options are OK, really. Aside from the other subprojects, I
think one interesting case to consider is when a user calls
get_maintainer.pl on tools/qemu-xen-dir, which should return a warning
or error because that's QEMU, not Xen.

So, I think it would be best to go with 2.2) introducing a new tag to
distinguish the Xen MAINTAINERS file from the QEMU MAINTAINERS file, so
that we can properly return a warning for tools/qemu-xen-dir, but at the
same time it could work fine for mini-os.git.
Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.