On Tue 2019-06-04 22:46:24 +0300, Tomi Ollila wrote: > - all variables in $((...)) without leading $ > - all comparisons use -gt, -eq or -ne > - no -a nor -o inside [ ... ] expressions > - all indentation levels using one tab > > Dropped unnecessary empty string check when reading results files. > > Replaced pluralize() which was executed in subshell with > pluralize_s(). pluralize_s sets $s to 's' or '' based on value of > $1. Calls to pluralize_s are done in context of current shell, so > no forks to subshells executed.
I'm fine with all these changes, but: > -if [ "$fixed" = "0" ] && [ "$failed" = "0" ]; then > +if [ $fixed -eq 0 ] && [ $failed -eq 0 ]; then I think we've set $fixed and $failed above to a non-empty string value, so this is technically correct. But the shell programmer nit-picker in me gets nervous seeing any variable used inside a test without proper wrapping, and i'm going to have trouble adopting $foo instead of "$foo" in other shell scripts. it seems to require a lot of global reasoning about the state of a given variable to use it without quotes safely, and it introduces some subtle requirements (like, no unsetting these variables and no setting them to the empty string). So anyway, i don't see the harm in using "$foo" instead in this case, and it seems like it actually reduces the cognitive burden of maintiaining the code. i'm happy to listen to any compelling story yo've got for why this is an important change, but i'd prefer to avoid it. (i don't at all mind unwrapping "0" to 0 though) thanks for doing this cleanup! --dkg
signature.asc
Description: PGP signature
_______________________________________________ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch