From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Usman Akinyemi <usmanakinyemi202@gmail.com>,
git@vger.kernel.org, christian.couder@gmail.com,
johncai86@gmail.com, me@ttaylorr.com, ps@pks.im,
shejialuo@gmail.com, phillip.wood123@gmail.com,
Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH v2 1/1] builtin/update-server-info: remove unnecessary if statement
Date: Mon, 07 Apr 2025 15:49:19 +0000 [thread overview]
Message-ID: <xmqqa58snf9c.fsf@gitster.g> (raw)
In-Reply-To: <CAPig+cShdouOzG_jKz_Z6+bSprZ5ZEsx9wZR-_LuD1P2kaOWwg@mail.gmail.com> (Eric Sunshine's message of "Sun, 6 Apr 2025 20:24:18 -0400")
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Sun, Apr 6, 2025 at 8:15 AM Usman Akinyemi
> <usmanakinyemi202@gmail.com> wrote:
>> Since we already teach the `repo_config()` in "1a764cdbdc
>> (Merge branch 'ua/some-builtins-wo-the-repository', 2025-03-26)"
>> to allow `repo` to be NULL, no need to check if `repo` is NULL
>> before calling `repo_config()`.
>
> Okay, makes sense. However...
>
> By referencing only the merge commit in the above message, you force
> reviewers and future readers to chase down and locate the actual
> commit[*] which taught repo_config() to accept NULL for `repo`.
Thanks for raising this.
When referring to an entire long series as a whole, a reference to
the concluding merge might be more useful, as the topic name
(hopefully) concisely summarizes what the topic was. However,
referring to a single patch series, or a single step in a longer
topic, ...
> To be
> more friendly to those people, you should help them by instead
> referencing the commit[*] itself.
... this is a useful general advice.
> [*]: f29f1990b5 (config: teach repo_config to allow `repo` to be NULL,
> 2025-03-08)
>
>> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
next prev parent reply other threads:[~2025-04-07 15:49 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-29 11:59 [PATCH 0/1] remove unnecessary if statement Usman Akinyemi
2025-03-29 11:59 ` [PATCH 1/1] builtin/update-server-info: " Usman Akinyemi
2025-03-31 7:16 ` Patrick Steinhardt
2025-04-06 12:14 ` [PATCH v2 0/1] " Usman Akinyemi
2025-04-06 12:14 ` [PATCH v2 1/1] builtin/update-server-info: " Usman Akinyemi
2025-04-07 0:24 ` Eric Sunshine
2025-04-07 15:49 ` Junio C Hamano [this message]
2025-04-07 19:58 ` [PATCH v2 0/1] " Usman Akinyemi
2025-04-07 19:58 ` [PATCH 1/1] builtin/update-server-info: " Usman Akinyemi
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=xmqqa58snf9c.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=johncai86@gmail.com \
--cc=me@ttaylorr.com \
--cc=phillip.wood123@gmail.com \
--cc=ps@pks.im \
--cc=shejialuo@gmail.com \
--cc=sunshine@sunshineco.com \
--cc=usmanakinyemi202@gmail.com \
/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.