From: Jonathan Nieder Date: Thu, 08 Dec 2011 17:12:13 GMT Subject: Re: [PATCH 5/6] t1510 (worktree): fix '&&' chaining Message-ID: <20111208171213.GF2394@elie.hsd1.il.comcast.net> In-Reply-To: <1323349817-15737-8-git-send-email-artagnon@gmail.com> Ramkumar Ramachandra wrote: > 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? > > Signed-off-by: Ramkumar Ramachandra > --- > t/t1501-worktree.sh | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) For what it's worth, after those changes, Acked-by: Jonathan Nieder