Re: [PATCHv2 10/19] git-submodule.sh: convert test -a/-o to && and ||
From: Matthieu Moy <hidden>
Date: 2016-06-15 23:01:17
Elia Pinto [off-list ref] 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 <redacted> 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, ...
quoted hunk
@@ -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. -- Matthieu Moy http://www-verimag.imag.fr/~moy/