Re: [PATCH 5/6] t1510 (worktree): fix '&&' chaining

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 8, 2011, 17:12 UTC
Message-ID: <20111208171213.GF2394@elie.hsd1.il.comcast.net>
In-Reply-To: <1323349817-15737-8-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra wrote:

Show 6 quoted lines

> Breaks in a test assertion's && chain can potentially hide failures
> from earlier commands in the chain.  Fix these breaks.
>
> Additionally, note that 'unset' returns non-zero status when the
> variable passed was already unset on some shells: change these
> instances to 'safe_unset'.

This is also not "Additionally", as in "As a separate change that maybe should have been another patch but I am too lazy". Rather, it is a necessary change that is part of the same task. So I would write:

	Breaks in a test assertion's && chain can potentially hide failures
	from earlier commands in the chain.  Fix these breaks.

	'unset' returns non-zero status when the variable passed was already
	unset on some shells, so now that the status is tested we need to
	change these instances to 'safe_unset'.

Erm, sane_unset, not safe_unset.  Did you even test this?

Show 5 quoted lines

>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t1501-worktree.sh |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

For what it's worth, after those changes,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>