Re: [PATCH 2/3] cat-file: add %(objectmode) atom

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 13, 2024, 21:23 UTC
Message-ID: <xmqqfrwtakrq.fsf@gitster.g>
In-Reply-To: <xmqq34swo1p2.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 29 quoted lines

>> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
>> index ac1f754ee32..6f25cc20ec6 100755
>> --- a/t/t1006-cat-file.sh
>> +++ b/t/t1006-cat-file.sh
>> @@ -114,9 +114,10 @@ run_tests () {
>>      type=$1
>>      object_name=$2
>>      oid=$(git rev-parse --verify $object_name)
>> -    size=$3
>> -    content=$4
>> -    pretty_content=$5
>> +    mode=$3
>> +    size=$4
>> +    content=$5
>> +    pretty_content=$6
>>  
>>      batch_output="$oid $type $size
>>  $content"
>
> I wonder if appending $mode as an optional thing at the end would
> have made the patch less noisy?  After all, the expectation above
> that does not have $mode, and the tests that are expected to produce
> output that match the expectation, do not have to change.  And the
> existing invocation of run_tests that do not care about $mode do not
> have to change.
>
> But I guess if the damage is only with the above 7-lines (which
> would become just 1 if we made mode the $6 last tthing), it is not a
> huge deal either way?

Unfortunately, not really.

If we made the optional mode as the last thing, and allow run_tests() to be called without an explicit "", it may have avoided unnecessary conflicts with eb/hash-transition topic. Interested folks can see how well these three patches plays with the other topic by trying to merge it to 'seen'.