René Scharfe <[email protected]> writes:

> +commit_and_tag () {
> +     message=$1 &&
> +     shift &&
> +     git add $@ &&

Lack of dq around $@ makes me wonder if there is something funny
going on (looking at the callers, there isn't, so we'd better quote
it to avoid wasting time, I think).

> +     test_tick &&
> +     git commit -m $message &&
> +     git tag $message
>  }

The use of $message as the sole argument to "git tag" makes the
readers guess that it must be a single token without any funny
character, so the readers would probably do not waste too much time
wondreing if the lack of dq around $message in the last two is
problematic.

> +last_context_line () {
> +     sed -n '$ p'
>  }

I have a vague recollection that some implementations of sed are
unhappy to see that space between the address and the operation; I'd
feel safer without it.

> +check_diff () {
> +     name=$1
> +     desc=$2
> +     options="-W $3"
> +
> +     test_expect_success "$desc" '
> +             git diff $options "$name^" "$name" >"$name.diff"
> +     '
> +
> +     test_expect_success ' diff applies' '
> +             test_when_finished "git reset --hard" &&
> +             git checkout --detach "$name^" &&

With the presence of ^ there, --detach is unnecessary; it would not
hurt, though.

> +             git apply "$name.diff" &&
> +             git diff --exit-code "$name"

Even though we may know that $name.diff" will never have a creation
of new paths, I'd feel safer if "apply" is run with "--index".


Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to