"nsengaw4c via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Nsengiyumva Wilberforce <nsengiyumvawilberforce@xxxxxxxxx>
>
> Tests in this script use an unusual and hard to reason about
> conditional construct
>
> if expression; then false; else :; fi
>
> Change them to use more idiomatic construct:
>
> ! expression
>
> Cc: Christian Couder <christian.couder@xxxxxxxxx>
> Cc: Hariom Verma <hariom18599@xxxxxxxxx>
> Signed-off-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@xxxxxxxxx>
What are these C: lines for? I do not think the message I am
responding to is Cc'ed to them. There may be a special incantation
to tell GitGitGadget to Cc to certain folks, but adding Cc: to the
log message trailer like this does not seem to be it---at least it
appears that it did not work that way.
> ...
> - if read_tree_u_must_succeed -m -u $treeH $treeM; then false; else :; fi'
> + ! read_tree_u_must_succeed -m -u $treeH $treeM'
Looks good. For the purpose of microproject, I think this is a good
place to stop, as it does not make anything worse and make the code
prettier.
To those more experienced contributors who are watching from
sidelines, and especially to our mentors, it may be worth taking a
look at the implementation of the helper shell function used here,
and think if it makes sense to expect a failure with a simple "!"
prefix (or with the original long hand if/then/else/fi that has
exactly the same issue).
read_tree_u_must_succeed () {
git ls-files -s >pre-dry-run &&
git diff-files -p >pre-dry-run-wt &&
git read-tree -n "$@" &&
git ls-files -s >post-dry-run &&
git diff-files -p >post-dry-run-wt &&
test_cmp pre-dry-run post-dry-run &&
test_cmp pre-dry-run-wt post-dry-run-wt &&
git read-tree "$@"
}
What if read-tree segfaults? This entire function will fail and the
test that runs read_tree_u_must_succeed and negates its result would
be a poor fit here.
Thanks.
