Re: [PATCH 2/2] Add transport message for up-to-date references

From: Martin Ågren <martin.agren@gmail.com>
Date: Mar 14, 2024, 21:44 UTC
Message-ID: <CAN0heSp2A-sx_WA__90WGhP1yD3EBBtR7_N7s5ZPWqe559xpKA@mail.gmail.com>
In-Reply-To: <SA1PR14MB4691A8AC86E290A3539BE1398D2B2@SA1PR14MB4691.namprd14.prod.outlook.com>

On Tue, 12 Mar 2024 at 22:55, Christopher Lindee <christopher.lindee@webpros.com> wrote:

Show 8 quoted lines

>
> If the `--send-up-to-date` option in the previous commit is used, the
> "Everything up-to-date!" message will never appear, even if all of the
> refs are up to date.  Moreover, the output `deadbeef..deadbeef` appears
> suspicious, almost as if a collision occurred.  To clarify that the hash
> is, in fact, identical & to allow grepping for the phrase "up-to-date",
> add a message to the output when the ref is transmitted, but no change
> occurred.

> +                       if (oideq(old_oid, new_oid))
> +                               msg = "up-to-date";

From a grammar point of view, I think this should be "up to date". (A branch can be up to date, in which case it is an up-to-date branch.)

There is quite some history exactly around this phrase, though, see, e.g., 80bdaba894 ("messages: mark some strings with "up-to-date" not to touch", 2024-01-12). If we really want "up-to-date" here, I think we should add a comment similar to 80bdaba894, although it does feel awkward to knowingly introduce a new instance. So maybe better, go for "up-to-date ref"?

If we worry about how "Everything up-to-date" disappeared in the previous commit, maybe that commit should instead detect that all refs were such no-ops and emit that "Everything up-to-date"?

(Part of me wonders what kind of existing scripts [1] would be helped by us sticking to the dashed form. If they go "grep '^Everything up-to-date$", they won't notice this new output. If they use a more relaxed "grep up-to-date", they might get confused, possibly much more often than they could already be by a branch called "up-to-date".)

Martin

[1] They don't even provide this new option, but let's assume we eventually grow a config knob.