git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Nipunn Koorapati via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Nipunn Koorapati <nipunn1313@gmail.com>,
	Alex Vandiver <alexmv@dropbox.com>
Subject: Re: [PATCH] dir.c: fix comments to agree with argument name
Date: Thu, 15 Oct 2020 11:41:36 -0700	[thread overview]
Message-ID: <xmqqk0vrfi1r.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20201015160725.GA1104947@coredump.intra.peff.net> (Jeff King's message of "Thu, 15 Oct 2020 12:07:25 -0400")

Jeff King <peff@peff.net> writes:

>> - * If "ss" is not NULL, compute SHA-1 of the exclude file and fill
>> + * If "oid_stat" is not NULL, compute SHA-1 of the exclude file and fill
>
> Makes sense. This changed as part of 4b33e60201 (dir: convert struct
> sha1_stat to use object_id, 2018-01-28). Perhaps it would likewise make
> sense to stop saying "SHA-1" here, and just say "hash" (or even "object
> id", though TBH I think the fact that the hash is the same as an
> object-id is largely an implementation detail).

I do not quite get your "though TBH", though.  I do agree with you
that it is an implementation detail that an object is named after
the hash of its contents, so saying "compute object name" probably
makes sense in more context than "compute hash" outside the narrow
parts of the code that actually implements how object names are
computed.  So I would have expected "because TBH", not "though TBH".

Anyway.  Nipunn, can you fix both of them in the same commit, as
they are addressing a problem from the same cause (i.e. we are no
longer SHA-1 centric).

Thanks.

  reply	other threads:[~2020-10-15 18:41 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-15 12:49 [PATCH] dir.c: fix comments to agree with argument name Nipunn Koorapati via GitGitGadget
2020-10-15 16:07 ` Jeff King
2020-10-15 18:41   ` Junio C Hamano [this message]
2020-10-15 19:33     ` Nipunn Koorapati
2020-10-15 19:41     ` Jeff King
2020-10-15 20:23       ` Junio C Hamano
2020-10-16 12:39         ` Nipunn Koorapati
2020-10-15 16:28 ` [PATCH v2] " Nipunn Koorapati via GitGitGadget

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=xmqqk0vrfi1r.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=alexmv@dropbox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=nipunn1313@gmail.com \
    --cc=peff@peff.net \
    /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).