Re: [PATCH 1/3] transport-helper: emit progress and verbosity options after asking for capabilities
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 12, 2015, 20:28 UTC
- Message-ID
- <xmqqlhk2ri1j.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1423735801-11108-1-git-send-email-mh@glandium.org>
Mike Hommey <mh@glandium.org> writes:
Show 6 quoted lines
> Currently, the progress and verbosity options are only emitted for the fetch > and push commands, but they should also be emitted for other commands, such as > import or export, and, why not, even list. > > Signed-off-by: Mike Hommey <mh@glandium.org> > ---
I had a hard time understanding what you are trying to achieve. The word "emit" may be techinically correct from the point of view of this program, but wouldn't there be a better word to describe what is being achieved by this program "emit"ting the additional output lines?
Perhaps "ask the helper to enable progress and verbosity capabilities" or somesuch?
They "should also be emitted" may also want to be justified in a better way, by describing what _bad_ things the current code that lacks this patch is doing as a bug.
Perhaps like "The current transport-helper.c code does not tell a remote helper that uses the import and export interface to enable progress and verbosity capabilities, even if the helper supports them."
Is there a downside of doing this? I do not think of any offhand.
Thanks.
Show 39 quoted lines
> transport-helper.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 0224687..23a741c 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -98,6 +98,8 @@ static void do_take_over(struct transport *transport)
> free(data);
> }
>
> +static void standard_options(struct transport *t);
> +
> static struct child_process *get_helper(struct transport *transport)
> {
> struct helper_data *data = transport->data;
> @@ -212,6 +214,7 @@ static struct child_process *get_helper(struct transport *transport)
> strbuf_release(&buf);
> if (debug)
> fprintf(stderr, "Debug: Capabilities complete.\n");
> + standard_options(transport);
> return data->helper;
> }
>
> @@ -339,7 +342,6 @@ static int fetch_with_fetch(struct transport *transport,
> int i;
> struct strbuf buf = STRBUF_INIT;
>
> - standard_options(transport);
> if (data->check_connectivity &&
> data->transport_options.check_self_contained_and_connected)
> set_helper_option(transport, "check-connectivity", "true");
> @@ -824,7 +826,6 @@ static int push_refs_with_push(struct transport *transport,
> return 0;
> }
>
> - standard_options(transport);
> for_each_string_list_item(cas_option, &cas_options)
> set_helper_option(transport, "cas", cas_option->string);