| Message ID | 1467357303-3433-1-git-send-email-czuzu@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 7/1/2016 10:15 AM, Corneliu ZUZU wrote: > Fix vm-event section of MAINTAINERS file. > > Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> > --- > MAINTAINERS | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e91140f..7bff878 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -402,10 +402,14 @@ M: Razvan Cojocaru <rcojocaru@bitdefender.com> > M: Tamas K Lengyel <tamas@tklengyel.com> > 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 > Re: Tamas & Razvan added to the CC list. Corneliu.
>>> On 01.07.16 at 09:15, <czuzu@bitdefender.com> 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. > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -402,10 +402,14 @@ M: Razvan Cojocaru <rcojocaru@bitdefender.com> > M: Tamas K Lengyel <tamas@tklengyel.com> > 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 Please at least don't make (un)sorted-ness worse than it was. Jan
On 7/1/2016 10:27 AM, Jan Beulich wrote: >>>> On 01.07.16 at 09:15, <czuzu@bitdefender.com> 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? > >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -402,10 +402,14 @@ M: Razvan Cojocaru <rcojocaru@bitdefender.com> >> M: Tamas K Lengyel <tamas@tklengyel.com> >> 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 > Please at least don't make (un)sorted-ness worse than it was. > > Jan 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?). Corneliu.
>>> On 01.07.16 at 09:40, <czuzu@bitdefender.com> wrote: > On 7/1/2016 10:27 AM, Jan Beulich wrote: >>>>> On 01.07.16 at 09:15, <czuzu@bitdefender.com> 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@bitdefender.com> >>> M: Tamas K Lengyel <tamas@tklengyel.com> >>> 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 >> 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
On 7/1/2016 11:02 AM, Jan Beulich wrote: >>>> On 01.07.16 at 09:40, <czuzu@bitdefender.com> wrote: >> On 7/1/2016 10:27 AM, Jan Beulich wrote: >>>>>> On 01.07.16 at 09:15, <czuzu@bitdefender.com> 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@bitdefender.com> >>>> M: Tamas K Lengyel <tamas@tklengyel.com> >>>> 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.
>>> On 01.07.16 at 11:12, <czuzu@bitdefender.com> wrote: > 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. Jan
On Jul 1, 2016 01:17, "Corneliu ZUZU" <czuzu@bitdefender.com> wrote: > > Fix vm-event section of MAINTAINERS file. > > Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> > --- > MAINTAINERS | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e91140f..7bff878 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -402,10 +402,14 @@ M: Razvan Cojocaru <rcojocaru@bitdefender.com > > M: Tamas K Lengyel <tamas@tklengyel.com> > S: Supported > F: xen/common/mem_access.c > -F: xen/*/vm_event.c > -F: xen/*/monitor.c Hm, I would think the above covers all vm_event.c and monitor.c files in all folders already, even in subfolders. Is that not how it's interpeted by the get_maintainers script? Tamas
>>> On 01.07.16 at 17:32, <tamas.k.lengyel@gmail.com> wrote: > On Jul 1, 2016 01:17, "Corneliu ZUZU" <czuzu@bitdefender.com> wrote: >> >> Fix vm-event section of MAINTAINERS file. >> >> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> >> --- >> MAINTAINERS | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index e91140f..7bff878 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -402,10 +402,14 @@ M: Razvan Cojocaru <rcojocaru@bitdefender.com >> >> M: Tamas K Lengyel <tamas@tklengyel.com> >> S: Supported >> F: xen/common/mem_access.c >> -F: xen/*/vm_event.c >> -F: xen/*/monitor.c > > Hm, I would think the above covers all vm_event.c and monitor.c files in > all folders already, even in subfolders. Is that not how it's interpeted by > the get_maintainers script? I don't think a * is meant to match a path separator. Jan
On Fri, Jul 1, 2016 at 9:55 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 01.07.16 at 17:32, <tamas.k.lengyel@gmail.com> wrote: >> On Jul 1, 2016 01:17, "Corneliu ZUZU" <czuzu@bitdefender.com> wrote: >>> >>> Fix vm-event section of MAINTAINERS file. >>> >>> Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> >>> --- >>> MAINTAINERS | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index e91140f..7bff878 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -402,10 +402,14 @@ M: Razvan Cojocaru <rcojocaru@bitdefender.com >>> >>> M: Tamas K Lengyel <tamas@tklengyel.com> >>> S: Supported >>> F: xen/common/mem_access.c >>> -F: xen/*/vm_event.c >>> -F: xen/*/monitor.c >> >> Hm, I would think the above covers all vm_event.c and monitor.c files in >> all folders already, even in subfolders. Is that not how it's interpeted by >> the get_maintainers script? > > I don't think a * is meant to match a path separator. > Ah indeed, I though it does when I made this change. It would be easier if it did so we wouldn't have to add each folder here separately, but anyhow: Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
diff --git a/MAINTAINERS b/MAINTAINERS index e91140f..7bff878 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -402,10 +402,14 @@ M: Razvan Cojocaru <rcojocaru@bitdefender.com> M: Tamas K Lengyel <tamas@tklengyel.com> 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
Fix vm-event section of MAINTAINERS file. Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> --- MAINTAINERS | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)