Re: [PATCH v4 4/4] builtin/stash: provide a way to import stashes from a ref

[Junio C Hamano <gitster@pobox.com>]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2022-04-12 at 20:14:34, Jonathan Tan wrote:
>> This seems like you're using the commit message as the reflog message -
>> is this necessary? For what it's worth, all tests still pass if I
>> replace "msg" with "NULL".
>
> I think that's what the existing stash code does, and so I did the same
> here.  It's not strictly necessary, but it's a nice to have.
>
> I didn't think it worth testing, because I don't think we test it
> elsewhere, either.
>
>> It might be worth adding tests that check that the exported stashes are
>> in the expected format (to ensure that we can read stashes exported from
>> another Git version) but I don't think that has to block the submission
>> of this patch set.
>
> There's a tiny patch for that for the base commit, but you're right that
> some more tests wouldn't hurt.  I can send a followup patch or two as
> part of a new series.

Is this about the log messages recorded in the throw-away commits
that are only used to form a single backbone chain, to which the
commits used to represent stash entries are linked to?

Are these messages meant to be used in any way?  I do not think
these messages contribute anything to the end result (they are just
discarded once they serve their purpose of transferring the
underlying stash entries, if I recall the design discussion
correctly), so I am not sure if we would even want to cast in stone
what they would say.

If on the other hand they are meant to be read by something (either
programs or end-user humans), it does make sense to ensure that we
are recording what we think we are recording.

Thanks.