Re: [PATCH 05/16] t2018: don't lose return code of git commands

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Denton Liu <liu.denton@xxxxxxxxx> writes:

>> 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?

I find Eric's slightly easier to follow in this particular case,
primarily because the problem being solved is so obvious (i.e. "so
that their exit codes are not lost" is sufficient to convey that the
current code lose exit codes and that it is bad to lose exit codes).

When the problem is deeper or needs longer backstory to understand
where it came from, I do prefer a separate description of the
current status and reason why the current status is undesirable
(both in the present tense), followed by commands to the code to
become better.

Thanks.



>> 
>> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux