Hi Eric,
On Fri, Dec 27, 2019 at 04:42:53PM -0500, Eric Sunshine wrote:
> On Fri, Dec 27, 2019 at 8:48 AM Denton Liu <liu.denton@xxxxxxxxx> wrote:
> > We had some git commands wrapped in non-assignment command substitutions
> > which would result in their return codes to be lost. Rewrite these
> > instances so that their return codes are now checked.
>
> Try writing your commit messages in imperative mood:
>
> Fix invocations of Git commands so their exit codes are not lost
> within command substitutions...
>
> or something. Same comment about several other commit messages in this series.
I thought that the preferred form of commit messages is to introduce the
problem that I'm trying to solve first ("We had some git commands losing
return codes") then, after that, describe the changes I made in
imperative mood ("Rewrite these instances..."). From what I can tell,
all of my commit messages conform to this template.
I'd prefer to keep the "problem statement" introduction in my commit
messages as it primes readers so they know "why" before "what" but I
can't think of any way to phrase the "problem statement" part in a way
that sounds good without resorting to past tense. Any suggestions?
>
> More below...
>
> > Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx>
> > ---
> > diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> > @@ -23,7 +23,8 @@ do_checkout () {
> > # if <sha> is not specified, use HEAD.
> > - exp_sha=${2:-$(git rev-parse --verify HEAD)} &&
> > + head=$(git rev-parse --verify HEAD) &&
> > + exp_sha=${2:-$head} &&
>
> Are you sure this transformation is needed? In my tests, the exit code
> of the Git command is not lost.
Thanks for double checking, it's not needed. I'll drop this in my next
reroll.
