[Linux-kernel-mentees] [PATCH] Documentation: MAINTAINERS: add information about parse-maintainers.pl

[Lukas Bulwahn]

On Sat, Nov 21, 2020 at 1:59 PM Aditya <yashsri421 at gmail.com> wrote:
>
> On 21/11/20 3:27 pm, Lukas Bulwahn wrote:
> > On Fri, Nov 20, 2020 at 11:16 AM Aditya <yashsri421 at gmail.com> wrote:
> >>
> >> On 20/11/20 3:42 pm, Aditya Srivastava wrote:
> >>> parse-maintainers.pl is a script which can be used to automatically
> >>> order the contents of MAINTAINERS file in preferred order (i.e.
> >>> 'MRLSWQBCPTFXNK').
> >>>
> >>> But currently we don't have a documentation on it.
> >>>
> >>> Add documentation regarding the same in MAINTAINERS file.
> >>>
> >>> Signed-off-by: Aditya Srivastava <yashsri421 at gmail.com>
> >>> ---
> >>> the patch applies perfectly on latest next-20201119
> >>>
> >>>  MAINTAINERS | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/MAINTAINERS b/MAINTAINERS
> >>> index 5db753fa1295..7ad00af7acd4 100644
> >>> --- a/MAINTAINERS
> >>> +++ b/MAINTAINERS
> >>> @@ -136,6 +136,9 @@ Descriptions of section entries and preferred order
> >>>             printk, pr_info or pr_err
> >>>          One regex pattern per line.  Multiple K: lines acceptable.
> >>>
> >>> +This order can be generated automatically for you using
> >>> +``scripts/parse-maintainers.pl``.
> >>> +
> >>>  Maintainers List
> >>>  ----------------
> >>>
> >>>
> >>
> >> Rationale for my changes:
> >> I did not go into much detail of its working here.
> >> A bit detail on this can be to use 'scripts/parse-maintainers
> >> --sorted' which produces the sorted file in MAINTAINERS.new by
> >> default. The input and output default files can be changed using
> >> '--input' and '--output' options. However, here I have omitted these
> >> details as running with '--help' option already provides this
> >> information in detail.
> >> So just making user aware of the script should serve our purpose.
> >>
> >> What do you think?
> >>
> >
> > It makes sense not to put that into MAINTAINERS.
> >
> > Do you understand how to use ./scripts/parse-maintainers, though?
> >
> Yes. From what I have understood, parse-maintainers.pl orders the
> maintainers lists in ASCII order.
>
> So, the list for '3C59X NETWORK DRIVER' will be displayed before
> '3CR990 NETWORK DRIVER'.
>
> It further orders the entries inside every list in this alphabetical
> order: 'MRLSWQBCPTFXNK'.
>
> > So if the warning with checkpatch is shown on a patch, what would you
> > need to execute to get a corrected patch?
> >
>
> We can directly use this command 'scripts/parse-maintainers.pl
> --output="MAINTAINERS" --order' to replace the MAINTAINERS with the
> correct order.
> However, I wouldn't recommend going for it. This is so because:
> 1) The MAINTAINERS file at that time might not be ordered properly for
> all the different lists in it or among each other. This will result
> into producing more numbers of changed lines as desired.
>
> For eg, currently MAINTAINERS is not ordered properly. Running
> parse-maintainers.pl on it results in placing 'DELL WMI NOTIFICATIONS
> DRIVER' before 'DELL WMI SYSMAN DRIVER' and so on. (producing changes
> with 112 Removals and 113 Additions, for simple ordering)
>
> However if we are sure that these misplaced orders should not be
> placed in that way, we can add a warning and fix for this too ie for
> mutual ASCII ordering of the lists.
> But this might need the list to be ordered once, otherwise for any
> further changes to MAINTAINERS, it will emit this warning.
>
> The best option currently though might be to use
> 'scripts/parse-maintainers.pl --order'. This creates a new
> MAINTAINERS.new file, without harming our 'MAINTAINERS' file.
> Now, one can check if their entries are correct manually and fix it
> accordingly.
> However, we can maybe automate this (as I think) by:
> Read MAINTAINERS.new and store these lists entries as hashes and
> replace it somehow (maybe using line numbers) in MAINTAINERS with the
> sorted one.
>
> > So if the warning with checkpatch is shown on a patch, what would ou
> > you need to execute to get a corrected patch?
>
> Again coming to this question, I don't think we can modify the patch
> alone, as the line numbers and other informations in the diff
> generated may change drastically. So, I think the user should probably
> consider making changes in MAINTAINERS itself and then creating patch.
>
> What do you think?
>

Yes, but we need to provide some help on how to do that... I have no
clue and there is no documentation :)

So, let us assume I create a patch where checkpatch warns I broke MAINTAINERS.

Which steps do I need to do? Can we---for the sake of common
understanding and making the workflow work smoothly---go together the
steps that need to be done?

I guess step 1, you create a quick patch that modifies a MAINTAINERS
section to be wrongly ordered, and send it to me to run checkpatch.pl
and apply it on my latest linux-next tree...

Lukas

> Thanks
> Aditya
>
> > Lukas
> >
>