Re: [PATCH v4 12/12] sequencer: notify user of --update-refs activity

From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee <derrickstolee@github.com>
Cc: Phillip Wood <phillip.wood@dunelm.org.uk>,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Taylor Blau <me@ttaylorr.com>,
	Jeff Hostetler <git@jeffhostetler.com>
Subject: Re: [PATCH v4 12/12] sequencer: notify user of --update-refs activity
Date: Sat, 16 Jul 2022 13:51:01 -0700	[thread overview]
Message-ID: <CABPp-BHjWQahfHMOvvGL2WdReCpHgfjF9Yp5DHPiKebk0iSHbg@mail.gmail.com> (raw)
In-Reply-To: <fc2e8f9d-0232-4c19-2e4d-9ba6c9b17ab2@github.com>

On Fri, Jul 15, 2022 at 6:20 AM Derrick Stolee <derrickstolee@github.com> wrote:
>
> On 7/15/2022 6:12 AM, Phillip Wood wrote:
>
> > Thanks for the comprehensive commit message and for implementing an excellent suggestion from Elijah. I wonder if it makes sense to distinguish between the current branch and all the others when writing the update message as we do here or if all the refs should just be in a single list. I also think it doesn't matter much and we can change it later if we want.
>
> I'm definitely open to suggestions, but I also think we should start
> somewhere and see what users think. Since the mechanisms for updating the
> refs are different, I felt it was appropriate to have different error
> messages.

I think the separate messages are fine, but I'm a little surprised by
the wording of this rationale.  The mechanisms for updating the refs
are implementational details that are likely not known by the end
user, and aren't something they can tweak either.  As such, I don't
think the mechanism used should result in any differences in end-user
messages here.

However, one difference here is that one of the refs' changes (namely,
the one corresponding to HEAD) will also have its changes be reflected
in the current working directory and index.  That may be a good reason
to keep its update message separate from the rest of the refs.

But on a related tangent...

I'm still curious if people are going to be surprised to not see an
"update-ref" line in the todo list for the HEAD ref; I was at first,
and only understood the rationale for excluding it based on the
implementational details.  I know as far as the implementation goes
that if such a line did appear in the todo list, then you'd probably
just ignore it (other then ensuring it was the last line) and then let
the pre-existing mechanisms in rebase update the HEAD ref...but even
with that implementation "weirdness", I think incorporating it might
be less surprising to the end user than the current behavior.  Or
maybe I'm just knee-deep in implementation details anyway, and I'm
just guessing what others might think.  I don't feel strongly about
it, just thought I'd surface the thought.