On Sat, Jul 30, 2022 at 2:44 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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.
