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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 14, 2024, 00:07 UTC
Message-ID: <xmqqttl94qxg.fsf@gitster.g>
In-Reply-To: <SA1PR14MB4691A8AC86E290A3539BE1398D2B2@SA1PR14MB4691.namprd14.prod.outlook.com>

Christopher Lindee <christopher.lindee@webpros.com> writes:

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

Same comment as [1/2].  Perhaps

    push: mark forced no-op pushes differently in the report

or something?

Show 26 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.
>
> Signed-off-by: Christopher Lindee <christopher.lindee@webpros.com>
> ---
>  transport.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/transport.c b/transport.c
> index 84deadd2b6..89802452ea 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -670,6 +670,8 @@ static void print_ok_ref_status(struct ref *ref,
>  			strbuf_addstr(&quickref, "..");
>  			type = ' ';
>  			msg = NULL;
> +			if (oideq(old_oid, new_oid))
> +				msg = "up-to-date";
>  		}
>  		strbuf_add_unique_abbrev(&quickref, new_oid,
>  					 DEFAULT_ABBREV);

This code is in an if/else cascade of this shape:

	if we are deleting
		print [deleted]
	else if they did not have any
		print [new]
	else {
		if we forced
		then
			prepare to say forced
		else
			prepare to say an unforced update
		say "updated" in a certain way
	}

The above addition looks somewhat out of place. I would understand if the addition happened like so instead:

diff --git i/transport.c w/transport.c
index df518ead70..bacef94b33 100644
--- i/transport.c
+++ w/transport.c
@@ -666,6 +666,8 @@ static void print_ok_ref_status(struct ref *ref,
 			strbuf_addstr(&quickref, "...");
 			type = '+';
 			msg = "forced update";
+		} else if (oideq(old_oid, new_oid)) {
+			... prepare to say "same but forced no-op" ...
 		} else {
 			strbuf_addstr(&quickref, "..");
 			type = ' ';


to make the new logic flow more like so:

		if we forced
		then
			prepare to say forced
		else if we were forced to tell no-op push
			prepare to say we did no-op
		else
			prepare to say an unforced update
		say "updated" in a certain way