All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Pablo Sabater" <pabloosabaterr@gmail.com>
To: "Junio C Hamano" <gitster@pobox.com>,
	"Pablo Sabater" <pabloosabaterr@gmail.com>
Cc: <git@vger.kernel.org>
Subject: Re: [PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info()
Date: Tue, 27 Oct 2026 21:00:01 +0000	[thread overview]
Message-ID: <DMFX8JPX16W8.9VBEJMQVF998@gmail.com> (raw)
In-Reply-To: <xmqqpkwvj7bj.fsf@gitster.g>

On Tue Oct 27, 2026 at 7:02 PM WET, Junio C Hamano wrote:
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
>> get_remote_info() receives the name of the remote and from there it
>> builds the transport and at the end closes it.
>>
>> To be able to reuse the connection for multiple batches in a subsequent
>> commit, it is needed to extract that responsibility from
>> get_remote_info() and instead have it in its caller,
>> parse_cmd_remote_object_info(), which will handle the batches.
>
> Makes sense.
>
>> While at it, rename the transport variable 'gtransport' to 'transport',
>> as it looks like a global when it's not.
>
> Good move.  We started (not particularly this program but the
> transport layer in general) with a global instance of transport
> object called gtransport, but the ones that we create on demand and
> release after we are done are of course local and it is misleading
> to use such a prefix.
>
>> +	if (!transport->smart_options)
>> +		die(_("smart options required"));
>
> Is this an end-user facing error message?  I am not sure a user who
> saw this message can figure out what went wrong or how to correct
> it.

I thought it would be better to say why it fails instead of a generic
error. If we drop the check it will die() later with "remote does not
support object-info".

Since the smart options check only die()s when the remote is a bundle,
I could keep it and change the message to "object-info is not supported
on bundles".

Regards,
Pablo

  reply	other threads:[~2026-10-27 21:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-27  3:16 [PATCH RFC 0/3] cat-file: make remote-object-info request in batches Pablo Sabater
2026-10-27  3:16 ` [PATCH RFC 1/3] cat-file: extract the transport ownership from get_remote_info() Pablo Sabater
2026-10-27 19:02   ` Junio C Hamano
2026-10-27 21:00     ` Pablo Sabater [this message]
2026-10-28  6:37       ` Junio C Hamano
2026-10-28 12:07         ` Pablo Sabater
2026-10-28 13:42           ` Junio C Hamano
2026-10-27  3:16 ` [PATCH RFC 2/3] cat-file: make remote-object-info request in batches Pablo Sabater
2026-10-27  3:16 ` [PATCH RFC 3/3] cat-file: add --remote-max-batch option Pablo Sabater
2026-10-27 19:12 ` [PATCH RFC 0/3] cat-file: make remote-object-info request in batches Junio C Hamano
2026-10-27 20:47   ` Pablo Sabater
2026-10-27 20:53     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DMFX8JPX16W8.9VBEJMQVF998@gmail.com \
    --to=pabloosabaterr@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.