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

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



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

 


Rackspace

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