All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: "Derick W. de M. Frias" <derick.william.moraes@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] t0200: replace 'test -d' with 'test_path_is_dir' and 'test -f' with 'test_path_is_file'
Date: Tue, 3 Jun 2025 08:16:04 +0200	[thread overview]
Message-ID: <aD6TJLuuaZmiFUvZ@pks.im> (raw)
In-Reply-To: <20250603053137.66249-1-derick.william.moraes@gmail.com>

On Tue, Jun 03, 2025 at 02:20:28AM -0300, Derick W. de M. Frias wrote:
> From: DerickWMFrias <derick.william.moraes@gmail.com>

There is a mismatch between your name here (which would be applied as
the patch author) and the Signed-off-by. These two should match.

> 'test_path_is_file' and 'test_path_is_dir' are modern path checking
> methods in Git's development.

We typically want to provide enough context in commit messages to state
_why_ the replacement is better. Them being more "modern" isn't yet a
sufficiently good reason. So it would be great if we explicitly mention
what the replacements bring to the table in a sentence or two.

> This patch replaces old 'test -d' and 'test -f' methods with them.

We use imperative style for commit messages, as if instructing the code
to change. So instead of saying "This patch replaces", we'd say
"Replace `test -d` and `test -f` ...".

The patch itself looks obviously good to me, thanks!

Patrick

  reply	other threads:[~2025-06-03  6:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-03  5:20 [PATCH] t0200: replace 'test -d' with 'test_path_is_dir' and 'test -f' with 'test_path_is_file' Derick W. de M. Frias
2025-06-03  6:16 ` Patrick Steinhardt [this message]
2025-06-18 18:31   ` [PATCH v2] " Derick W. de M. Frias

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aD6TJLuuaZmiFUvZ@pks.im \
    --to=ps@pks.im \
    --cc=derick.william.moraes@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.