Re: [PATCH v2 0/6] update-ref: add symref support for --stdin

From: Karthik Nayak <karthik.188@gmail.com>
Date: May 23, 2024, 17:50 UTC
Message-ID: <CAOLa=ZQq_ce8thcjxMitDt8s0aiqVUWt8A43ziU0Zd5F2Joorg@mail.gmail.com>
In-Reply-To: <xmqqr0dsv6e8.fsf@gitster.g>

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

Show 35 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Karthik Nayak <karthik.188@gmail.com> writes:
>>>
>>>> The patches 1, 5 fix small issues in the reference backends. The other
>>>> patches 2, 3, 4 & 6, each add one of the new sub-commands.
>>>>
>>>> The series is based off master, with 'kn/ref-transaction-symref' merged
>>>> in. There seem to be no conflicts with 'next' or 'seen'.
>>>
>>> Wait.  There is something fishy going on.
>>> ...
>>> Is this actually a single patch submission of 9/9 alone?  Patches
>>> 1-8/9 are all old ones that are in 'master' already.
>>>
>>> Puzzled...
>>
>> I think this is just a mess up in the range diff, I haven't changed
>> anything locally. So adding the correct range diff here:
>
> Quite honestly, I care much less about the range-diff that is almost
> unintelligible than the actual patches.  Your title line says 0/6,
> your updated range-diff presumably have 1: to 6:?  As a sanity check
> mechanism, the list of commits and the overall diffstat is a more
> useful part in the cover letter message so that I (or any other
> recipients) can use to compare against the list of messages that
> appeared on the list.
>
> We may want to teach "format-patch --range-diff" to place the output
> of range-diff _below_ the list of commits and the overall diffstat
> in the cover letter (and at the end of the patch for a single patch
> topic).
>

I usually manually add in the range-diff, which is probably where the error came from. I didn't even know about "format-patch --range-diff".

> I'll ignore the range-diff in the original cover letter and see if
> the rest makes sense.
>
> Thanks.

It does use the same base as the previous revision, I rebased in place using 'rebase -i' and amended for fixes from the first review.

Show 24 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Wait.  There is something fishy going on.
>>
>>> Range diff vs v1:
>>>  1:  1bc4cc3fc4 =  1:  1bc4cc3fc4 refs: accept symref values in `ref_transac...
>>>...
>>>  8:  4865707bda =  8:  4865707bda refs: remove `create_symref` and associated dead code
>>>  9:  4cb67dce7c !  9:  2bbdeff798 refs: create and use `ref_update_ref_must_exist()`
>> ...
>> I am confused why we are seeing a total reroll of such an old topic.
>>
>> Also you have one more patch at the end.  Neither the before or
>> after version of 9/9.
>>
>> Is this actually a single patch submission of 9/9 alone?  Patches
>> 1-8/9 are all old ones that are in 'master' already.
>
> And then there is a mystery of this v2 being a 6-patch series.
> Perhpas a wrong range-diff was pasted into it?  If this were truly a
> total reroll of the previous 8-patch series with an extra step
> appended to the end, it would have been a 9-patch series, not 6.
>
> Even puzzled...

The v1 of this series is also a 6-patch series, this is not a re-roll of the earlier series 'kn/ref-transaction-symref' (which is already in next). This is based on top of it.

Sorry for the confusion though.