[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH 1/9] t1416: more testcases for reference-transaction hook
From:       Jiang Xin <worldhello.net () gmail ! com>
Date:       2022-07-31 3:25:05
Message-ID: CANYiYbHcd3UNF2vq6YLQifEWW6eSLhB=7d23907TAbUeum2w0A () mail ! gmail ! com
[Download RAW message or body]

On Sat, Jul 30, 2022 at 2:44 PM Eric Sunshine <sunshine@sunshineco.com> wrote:

> Now that the function breaks out of the loop properly with `|| return
> 1` upon failure, it's no longer necessary to perform the directory
> check at the top of the function since the call to test_commit() will
> correctly fail if the directory does not exist. So, the function can
> be shortened to:
>
>     create_commits_in () {
>         local repo="$1" &&
>         shift &&
>         while test $# -gt 0
>         do
>             local name=$1 &&
>             shift &&
>             test_commit -C "$repo" --no-tag "$name" &&
>             local rev=$(git -C "$repo" rev-parse HEAD) &&
>             eval $name=$rev || return 1
>         done
>     }

Thanks, Eric. Appreciate for all suggestions. I prefer this version
and will use it in reroll v2.

> > +       done &&
> > +       expect=${1:-expect} &&
> > +       actual=${2:-actual-heads-and-tags} &&
> > +       indir=${indir:+"$indir"/} &&
> > +       test_path_is_file "$expect" &&
> > +       test_when_finished "rm -f \"$actual\"" &&
> > +       git ${indir:+ -C "$indir"} show-ref --heads --tags | \
> > +               make_user_friendly_and_stable_output >"$actual" &&
>
> The exit code from `git show-ref` is being lost down the pipe. You
> also don't need the `\` after `|`.

Will remove unnecessary backslash before EOL.

> > +       test_cmp "$expect" "$actual"
> > +}
> > +
> > +test_expect_success 'setup git config and common reference-transaction hook' '
> > +       git config --global \
> > +               core.hooksPath "$HOME/test-hooks" &&
>
> Nit: This would fit nicely on a single line; no need for the line splicing.

Will write in oneline.

> > +       git config --global core.abbrev 7 &&
> > +       mkdir "test-hooks" &&
> > +       write_script "test-hooks/reference-transaction" <<-EOF
> > +               exec >>"$HOME/$HOOK_OUTPUT"
> > +               printf "## Call hook: reference-transaction %9s ##\n" "\$@"
> > +               while read -r line
> > +               do
> > +                   printf "%s\n" "\$line"
>
> Nit This is the same as:
>
>     echo "\$line"

Will do.

> > +               done
> > +       EOF
> > +'
> > +
> > +test_expect_success "update-ref: create new refs" '
> > +       test_when_finished "rm -f $HOOK_OUTPUT" &&
> > +
> > +       cat >expect <<-EOF &&
> > +               ## Call hook: reference-transaction  prepared ##
>
> This and a bunch of other here-doc tags in subsequent tests are
> missing the backslash:
>
>     cat >expect <<-\EOF &&

Will add backslash unless there are variables need to be expanded.
[prev in list] [next in list] [prev in thread] [next in thread]