Re: [PATCH 1/2] t1450: robustify `remove_object()`
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 11, 2021, 00:10 UTC
- Message-ID
- <xmqqsg63zcpl.fsf@gitster.c.googlers.com>
- In-Reply-To
- <YCRqOVS/iXp/d6Rc@nand.local>
Taylor Blau <me@ttaylorr.com> writes:
Show 31 quoted lines
> On Wed, Feb 10, 2021 at 12:36:19PM -0800, Junio C Hamano wrote:
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>> writes:
>>
>> > -test_expect_success 'setup: helpers for corruption tests' '
>> > - sha1_file() {
>> > - remainder=${1#??} &&
>> > - firsttwo=${1%$remainder} &&
>> > - echo ".git/objects/$firsttwo/$remainder"
>> > - } &&
>> > +sha1_file () {
>> > + git rev-parse --git-path objects/$(test_oid_to_path "$1")
>> > +}
>>
>> Yeah, back when 90cf590f (fsck: optionally show more helpful info
>> for broken links, 2016-07-17) originally introduced this pattern,
>> we didn't have nicely abstracted helper, but now we do, and there
>> is no reason not to use it. Nice.
>
> This has nothing to do with this series, but I do notice a number of
> other uses of test_oid_to_path that are doing this exact thing. In fact,
> many of them don't use "git rev-parse --git-path", which would be
> better.
>
> I wonder if it's worth a clean-up on top to consolidate all of those
> "combine the loose object path of the object with xyz OID and the
> $GIT_DIR/objects directory".
>
> In either case -- and I think I'm pretty clearly being pedantic at this
> point -- do you suppose it's worthwhile to rename sha1_file to something
> that doesn't have sha1 in it?Possibly. That is probably outside the scope of this topic, but we see such SHA -> HASH clean-up patches in different places, and this certainly is a fair game for such a clean-up, I would think.
Thanks.