Re: [PATCHv2 10/19] git-submodule.sh: convert test -a/-o to && and ||

> Elia Pinto <gitter.spiros@gmail.com> writes:
>
>> This is version 2 of the patch to git-submodule of the
>> patch series "convert  test -a/-o to && and ||".
>> It contains the fixes identified by Johannes and
>> Matthieu.
>
> This version of the patch (not the whole series) is
>
> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> Some comments for next time:
>
> * I agree with Johannes that splitting the series with one patch per
>   file is not the right way to split. As a reviewer, I want to
>
>   - check that -a trivially translates to &&
>   - check that -o trivially translates to ||
>   - check non-trivial cases
>
>   Interleaving these cases (especially the trivial and non-trivial
>   cases) makes the review much harder. For this kind of series, the
>   change is trivial, but the review is not (Johannes caught a || Vs &&
>   inversion that I didn't find for example, which is quite serious), so
>   the "optimize your patches for review" is even more important than
>   usual.
>
> * Again, to avoid mixing trivial and non-trivial changes, ...

>
>> @@ -1059,13 +1059,17 @@ cmd_summary() {
>>               while read mod_src mod_dst sha1_src sha1_dst status sm_path
>>               do
>>                       # Always show modules deleted or type-changed (blob<->module)
>> -                     test $status = D -o $status = T && echo "$sm_path" && continue
>> +                     case "$status" in
>> +                     [DT])
>> +                             printf '%s\n' "$sm_path" &&
>> +                             continue
>> +                     esac
>
> turning a echo into a printf is good, but would be better done
> separately.