Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Philippe Mathieu-Daudé]

On Thu, Nov 8, 2018 at 3:30 PM Laurent Vivier <address@hidden> wrote:
> On 08/11/2018 11:24, Stefan Hajnoczi wrote:
> > On Tue, Nov 06, 2018 at 05:16:14PM +0100, Laurent Vivier wrote:
> >> On 06/11/2018 16:15, Philippe Mathieu-Daudé wrote:
> >>> On 6/11/18 15:13, Michael S. Tsirkin wrote:
> >>>> On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
> >>>>> On 5/11/18 19:14, Michael S. Tsirkin wrote:
> >>>>>> From: Peter Maydell <address@hidden>
> >>>>>>
> >>>>>> Currently tests/hex-loader-check-data contains data files used
> >>>>>> by the hexloader-test, and configure individually symlinks those
> >>>>>> data files into the build directory using a wildcard.
> >>>>>>
> >>>>>> Using a wildcard like this is a bad idea, because if a new
> >>>>>> data file is added, nothing causes configure to be rerun,
> >>>>>> and so no symlink is added for the new file. This can cause
> >>>>>> tests to spuriously fail when they can't find their data.
> >>>>>> Instead, it's better to symlink an entire directory of
> >>>>>> data files. We already have such a directory: tests/data.
> >>>>>>
> >>>>>> Move the data files from tests/hex-loader-check-data/ to
> >>>>>> tests/data/hex-loader/, and remove the unnecessary symlinking.
> >>>>>>
> >>>>>> Signed-off-by: Peter Maydell <address@hidden>
> >>>>>
> >>>>> I reviewed/tested this patch too.
> >>>>
> >>>>
> >>>> Thanks a lot Philippe!
> >>>> It is unfortunately too late to update this patch info in git
> >>>> commit history, however your help is still greatly appreciated!
> >>>
> >>> No worry, I'm not mad at all, but there might be an issue in your git PR
> >>> workflow, this series also missed your maintainer S-o-b.
> >>>
> >>> Peter: Can you add a such check in your scripts? (during next merge
> >>> window, no hurry).
> >>>
> >>> Rather than your scripts, this should be in scripts a maintainer can run
> >>> locally, such ./scripts/checkpatch.pl --maintainer or
> >>> ./scripts/checkseries.xx.
> >>
> >> I think such tool already exists: with git-publish you can configure a
> >> "pre-publish-send-email" hook, and check your S-o-B is present.
> >
> > For making my own commits .git/hooks/pre-commit is useful:
> > http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
> >
> > For sending patch series, git-publish is useful:
> > https://github.com/stefanha/git-publish/blob/master/hooks/pre-publish-send-email.example
> > https://github.com/stefanha/git-publish/
>
> A pre-publish-send-email like this should do the trick:
>
> #!/bin/bash
>
> NAME=$(git config --get user.name)
> EMAIL=$(git config --get user.email)
>
> for PATCH in $1/*.patch; do
>     if [ $(basename $PATCH) = "0000-cover-letter.patch" ]; then
>         continue
>     fi

Maybe easier (to handle series versions > 1):

    case ${PATCH} in
    *-cover-letter.patch)
        continue
        ;;
    *)
    ...

>     if ! grep -q "^Signed-off-by: $NAME <$EMAIL>" $PATCH; then
>         echo "Error: Missing sender S-o-B in $PATCH"
>         exit 1
>     fi
> done
> exit 0
>
>