git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "AreaZR via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,  AreaZR <gfunni234@gmail.com>,
	 Seija Kijin <doremylover123@gmail.com>
Subject: Re: [PATCH v4] git: replace greater-than and less-than checks with one not equal check
Date: Wed, 18 Dec 2024 07:39:49 -0800	[thread overview]
Message-ID: <xmqqjzbxt2yi.fsf@gitster.g> (raw)
In-Reply-To: <pull.1432.v4.git.git.1734489859673.gitgitgadget@gmail.com> (AreaZR via GitGitGadget's message of "Wed, 18 Dec 2024 02:44:19 +0000")

"AreaZR via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  	if (top_index[0] == htonl(PACK_IDX_SIGNATURE)) {
>  		version = ntohl(top_index[1]);
> -		if (version < 2 || version > 2)
> +		if (version != 2)
>  			die("unknown index version");

I am of two minds.  If the code never evolves and we will never
support anything other than version #2, your rewrite certainly makes
it easier to read.  On the other hand, if we plan to ever learn to
grok versions #3 and later, the original would be easier to se what
is going on, i.e.

		if (version < VERSION_LB || VERSION_UB < version)
			die("version out of bounds");

and the code as written happens to have "2" as both lower- and
upper-bound.

Of course when we do introduce version #3, this line must be updated
anyway, but the final form would be as we have it with the second
"2" replaced with "3", so leaving it in the current shape may be
easier for the developer doing that work.

So I do not know if the proposed change is an improvement for the
longer term.

      reply	other threads:[~2024-12-18 15:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-09 18:57 [PATCH] git: replace two checks with one not equal check Rose via GitGitGadget
2024-12-18  0:38 ` [PATCH v2] " AreaZR via GitGitGadget
2024-12-18  0:50   ` [PATCH v3] git: replace greater-than and less-than " AreaZR via GitGitGadget
2024-12-18  2:44     ` [PATCH v4] " AreaZR via GitGitGadget
2024-12-18 15:39       ` Junio C Hamano [this message]

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=xmqqjzbxt2yi.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=doremylover123@gmail.com \
    --cc=gfunni234@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).