Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:
> Junio C Hamano wrote:
>> If you are introducing "dotest exists but next/last may not be
>> present" as a valid new state, it probably should check the presence
>> and/or absence of them explicitly,
>
> Um, a test -f preceding the cat? Why, when cat implicitly checks
> existence anyway?
With that logic, 'test -d "$dotest"' on the first line would also be
redundant, as we read from "$dotest/last" and that will catch an
error.
It is more about what is conceptually right. You need to think what
the condition _means_.
We protect the "remove $dotest directory" with the precondition you
are changing, but what is that precondition really trying to say?
If $dotest does not exist, obviously we do not need to remove it,
but what is the essence of that sereis of tests?
It is what the comment says:
"We have finished applying all the patches in them"
Earlier, presense of $dotest _must_ have meant that last and next
should exist (otherwise you have a corrupt state and we did want to
see error messages), and the check original did was perfectly fine.
Now, a mere presense of $dotest does _not_ mean "we have finished
applying all the patches", because sometimes you create it without
having anything to put in last or next yet. That is why it starts
to make sense to do
if test -d "$dotest" &&
test -f "$dotest/last" &&
test -f "$dotest/next" &&
last=$(cat "$dotest/last") &&
...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html