Re: [PATCH] connect: use "service" enum for "name" argument

[Patrick Steinhardt <ps@pks.im>]

On Tue, May 19, 2026 at 01:22:19AM -0400, Jeff King wrote:
> diff --git a/connect.h b/connect.h
> index 1645126c17..c56ecddc0e 100644
> --- a/connect.h
> +++ b/connect.h
> @@ -7,7 +7,12 @@
>  #define CONNECT_DIAG_URL      (1u << 1)
>  #define CONNECT_IPV4          (1u << 2)
>  #define CONNECT_IPV6          (1u << 3)
> -struct child_process *git_connect(int fd[2], const char *url, const char *name, const char *prog, int flags);
> +enum git_connect_service {
> +    GIT_CONNECT_UPLOAD_PACK,
> +    GIT_CONNECT_RECEIVE_PACK,
> +    GIT_CONNECT_UPLOAD_ARCHIVE,
> +};
> +struct child_process *git_connect(int fd[2], const char *url, enum git_connect_service, const char *prog, int flags);
>  int finish_connect(struct child_process *conn);
>  int git_connection_is_socket(struct child_process *conn);
>  int server_supports(const char *feature);

This is all quite tightly-packed, and the patch would be a good
opportunity to maybe add some documentation. But that's certainly
moving the goalposts quite a bit.

> diff --git a/transport-helper.c b/transport-helper.c
> index 4614036c99..bf37c5280c 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -620,8 +620,22 @@ static int run_connect(struct transport *transport, struct strbuf *cmdbuf)
>  	return ret;
>  }
>  
> +static const char *connect_service_cmd(enum git_connect_service service)
> +{
> +	switch (service) {
> +	case GIT_CONNECT_UPLOAD_PACK:
> +		return "git-upload-pack";
> +	case GIT_CONNECT_RECEIVE_PACK:
> +		return "git-receive-pack";
> +	case GIT_CONNECT_UPLOAD_ARCHIVE:
> +		return "git-upload-archive";
> +	}
> +	BUG("unknown git_connect_type: %d", service);
> +}

Shouldn't this say "unknown git_connect_service" instead of "_type"?

Other than that this patch looks good to me, and I agree that this makes
the argument a bit easier to understand.

Patrick