Re: [PATCH v2 4/5][Outreachy] t7201: avoid using cd outside of subshells

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 19, 2020, 20:25 UTC
Message-ID: <20201019202456.GC42778@nand.local>
In-Reply-To: <CAPSFM5fvBt+x840XOwzwPBvXK7_1qB-sb+_M3LoPuKv_P=VvDA@mail.gmail.com>

Hi Charvi,

On Mon, Oct 19, 2020 at 10:54:57PM +0530, Charvi Mendiratta wrote:

Show 8 quoted lines

> Yes, thanks a lot Philip I understood the reason. I will do the corrections in
> commit message and commit body as below :
> t7201: using 'git -C' to avoid subshell
>
> Using 'git-C' instead of 'cd' inside of subshell, to avoid the extra process
> of starting a new subshell
>
> Please confirm, if any other changes are required .

Usually it never hurts to just send the patch, since any feedback that a reviewer has now is equally good even after you have sent a patch. Plus, it's easier to review the concrete patch you want applied, instead of a hypothetical of what you might send.

That said, a couple of notes:

  - Your subject message is good. It is concise, to-the-point, and
    accurately describes the change. Good.

  - The body is similarly short, but could be rewritten to use the
    imperative mood. But, it is redundant with the subject. The subject
    says "we are using 'git -C' to avoid creating a subshell", and the
    patch says exactly the same.

...So, you can do one of two things. Either you can abbreviate the subject, adding the additional detail in the patch message, or you could leave the subject as-is and delete the patch message entirely.

Either would be fine with me, but certainly Phillip or others could chime in, too.

Thanks, Taylor