Re: [Xen-devel] [PATCH] vm-event: MAINTAINERS fix

[Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>]

On 7/1/2016 11:02 AM, Jan Beulich wrote:
> > > > On 01.07.16 at 09:40, <czuzu@xxxxxxxxxxxxxxx> wrote:
> > On 7/1/2016 10:27 AM, Jan Beulich wrote:
> > > > > > On 01.07.16 at 09:15, <czuzu@xxxxxxxxxxxxxxx> wrote:
> > > > Fix vm-event section of MAINTAINERS file.
> > > Would be nice to mention here which commit(s) caused these to go
> > > out of sync with the actual code.
> > Why?
> Well, I already said so above - it would be nice (for reference,
> obviously).
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -402,10 +402,14 @@ M:        Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> > > >    M:   Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> > > >    S:   Supported
> > > >    F:   xen/common/mem_access.c
> > > > -F:     xen/*/vm_event.c
> > > > -F:     xen/*/monitor.c
> > > > +F:     xen/common/vm_event.c
> > > > +F:     xen/arch/*/vm_event.c
> > > > +F:     xen/common/monitor.c
> > > > +F:     xen/arch/*/monitor.c
> > > > +F:     xen/arch/x86/hvm/monitor.c
> > > >    F:   xen/include/*/mem_access.h
> > > >    F:   xen/include/*/monitor.h
> > > > +F:     xen/include/asm-x86/hvm/monitor.h
> > > >    F:   xen/include/*/vm_event.h
> > > >    F:   tools/tests/xen-access
'Culprits':

c/s ca63cee: "monitor: Rename hvm/event to hvm/monitor": adds 
arch/x86/hvm/monitor.c & asm-x86/hvm/monitor.h
c/s ec89da2: "MAINTAINERS: update monitor/vm_event covered code": I'm 
guessing it mistakenly (was Acked though) removes both 
arch/x86/monitor.c & arch/x86/vm_event.c
Will include in v2.

> > > Please at least don't make (un)sorted-ness worse than it was.
> > Please at least mention how these were sorted (/unsorted). They are
> > currently sorted, just not alphabetically (common before arch, grouped
> > by file, .c before .h).
> > How do you want me to sort them? Be specific (and don't make me guess
> > the rules, where is this written? if not written anywhere, how should I
> > adjust CODE_STYLE to fix that?).
> I agree that there are many violations of the (alphabetical) sorting
> we try to aim at. I'm sorry that I didn't say "alphabetical", as I've
> assumed people contributing would be reading other xen-devel
> traffic concerning similar areas.
>
> Jan
If you aim at that, make (diff-reliant) tools that automatically do this 
kind of superficial checks and instruct all contributors to run those 
checks before sending any patches.
That would at least avoid this contributor-reviewer back-and-forth for 
operations of little importance.
Other people's xen-devel contributions updating MAINTAINERS don't much 
concern me (or other xen-devel contributors) unless the diff in its 
entirety is of interest.
If nobody has time for making such tools, @ least specify in CODE_STYLE 
these rules or again @ least be polite when telling contributors to 
follow these guidelines if they're not written anywhere.
A lot of such minor checks can be done automatically, e.g.:
- remind contributor to check whether MAINTAINERS needs updating when 
his c/s adds new files
- check if entries in MAINTAINERS are (alphabetically) sorted
- check #includes list: alphabetically sorted, not repeating etc.
- validate formatting of source files (4 spaces instead of tabs, no 
spaces @ end of line, local-variables block @ EOF, #include guards 
according to full file path)
- even more advanced stuff like: check for unused includes
and even having the ability to make the needed changes automatically. 
Maybe I'll give a look into that these days.
Anyway, so you want me to sort those strictly alphabetically, i.e. the 
list would finally look like:
F:      tools/tests/xen-access
F:      xen/arch/*/monitor.c
F:      xen/arch/*/vm_event.c
F:      xen/arch/x86/hvm/monitor.c
F:      xen/common/mem_access.c
F:      xen/common/monitor.c
F:      xen/common/vm_event.c
F:      xen/include/*/mem_access.h
F:      xen/include/*/monitor.h
F:      xen/include/*/vm_event.h
F:      xen/include/asm-x86/hvm/monitor.h

, yes?

Thanks,
Corneliu.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel