| Message ID | 20170728053610.15770-48-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > scripts/check_maintainer.sh | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > create mode 100755 scripts/check_maintainer.sh > > diff --git a/scripts/check_maintainer.sh b/scripts/check_maintainer.sh > new file mode 100755 > index 0000000000..074a6acf69 > --- /dev/null > +++ b/scripts/check_maintainer.sh > @@ -0,0 +1,21 @@ > +#! /bin/bash > +# > +# This script checks MAINTAINERS consistency Consistency? I think you mean coverage. > +# > +# Copyright (C) 2017 Philippe Mathieu-Daudé. GPLv2+. > +# > +# Usage: > +# ./scripts/check_maintainer.sh | tee MAINTAINERS.missing > + > +echo "Incorrect MAINTAINERS paths:" 1>&2 > +egrep ^F: MAINTAINERS | cut -d\ -f2 | while read p; do > + ls -ld $p 1>/dev/null > +done > + > +echo "No maintainers found for:" 1>&2 > +git ls-files|while read f; do > + OUT=$(./scripts/get_maintainer.pl -f $f 2>&1) > + if [[ "$OUT" == *"No maintainers found"* ]]; then > + echo $f > + fi > +done Good idea. But what I really want is checkpatch whining when it happens. Thomas posted a patch some time ago. Would you be willing to revive it? https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05742.html Message-Id: <1485436265-12573-5-git-send-email-thuth@redhat.com> Basically: * Patch deletes a file - still in MAINTAINERS after the patch: warn - else: ok * Patch creates a file - not in in MAINTAINERS after the patch: warn - else: ok * Patch moves a file - old still in MAINTAINERS after the patch: warn - new not in in MAINTAINERS after the patch: warn - neither: ok May have to ignore "uninteresting" files to reduce the noise. But even ignoring everything but *.[ch] would be an improvement!
On 07/28/2017 11:26 AM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <f4bug@amsat.org> writes: > >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> +# This script checks MAINTAINERS consistency > > Consistency? I think you mean coverage. Indeed - consistency implies even more, such as all email addresses and git trees are still valid. Coverage is merely that all files at least have one listed owner, whether or not that owner is still correct. > But what I really want is checkpatch whining when it happens. Thomas > posted a patch some time ago. Would you be willing to revive it? > > https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05742.html > Message-Id: <1485436265-12573-5-git-send-email-thuth@redhat.com> > > Basically: > > * Patch deletes a file > > - still in MAINTAINERS after the patch: warn > - else: ok > Careful, since we have globs in MAINTAINERS. I guess it really means: * Patch deletes a file: - check all lines in MAINTAINERS; if any no longer maps to a file: warn - else: ok > * Patch creates a file > > - not in in MAINTAINERS after the patch: warn > - else: ok > > * Patch moves a file Same as delete + move, even if it shows up in git differently. > > May have to ignore "uninteresting" files to reduce the noise. But even > ignoring everything but *.[ch] would be an improvement! And like all checkpatch warnings, maintainers (or, in this case, lack of maintainer?) can ignore the warning and submit pull request anyway. But the key part is that by making it part of checkpatch, coupled with patchew CI, the whole list would know who's ignoring the warnings :)
On 07/28/2017 12:36 AM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > scripts/check_maintainer.sh | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > create mode 100755 scripts/check_maintainer.sh > > +echo "Incorrect MAINTAINERS paths:" 1>&2 > +egrep ^F: MAINTAINERS | cut -d\ -f2 | while read p; do > + ls -ld $p 1>/dev/null > +done Ah, you uses this in a cover letter of your other series, where I responded without thinking it was going into git. But now that I see we are trying to commit it, I really think that one 'ls -l >/dev/null' per filename is inefficient; this should be egrep ^F: MAINTAINERS | cut -d\ -f2 | xargs ls -d >/dev/null if we really like the side effect of ls writing to stdout as the way to identify globs that no longer map anywhere, or even something like: for glob in $(egrep ... | cut ...); do set - $glob if test -n "$2" && ! test -e "$1"; then echo "glob $glob no longer resolves" 2>&1 fi done
diff --git a/scripts/check_maintainer.sh b/scripts/check_maintainer.sh new file mode 100755 index 0000000000..074a6acf69 --- /dev/null +++ b/scripts/check_maintainer.sh @@ -0,0 +1,21 @@ +#! /bin/bash +# +# This script checks MAINTAINERS consistency +# +# Copyright (C) 2017 Philippe Mathieu-Daudé. GPLv2+. +# +# Usage: +# ./scripts/check_maintainer.sh | tee MAINTAINERS.missing + +echo "Incorrect MAINTAINERS paths:" 1>&2 +egrep ^F: MAINTAINERS | cut -d\ -f2 | while read p; do + ls -ld $p 1>/dev/null +done + +echo "No maintainers found for:" 1>&2 +git ls-files|while read f; do + OUT=$(./scripts/get_maintainer.pl -f $f 2>&1) + if [[ "$OUT" == *"No maintainers found"* ]]; then + echo $f + fi +done
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- scripts/check_maintainer.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100755 scripts/check_maintainer.sh