Nguyễn Thái Ngọc Duy <pclo...@gmail.com> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> ---
Just like other patches, this is thin on the rationale. You are
planning to invent another function that prepares the requrest in a
strbuf and allow it to call this helper to perform a single round
trip with the other side? If that is the case, the split is very
sensible, but you need to say something about "why".
> transport-helper.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/transport-helper.c b/transport-helper.c
> index a6bff8b..35023da 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -260,6 +260,28 @@ static const char *boolean_options[] = {
> TRANS_OPT_FOLLOWTAGS,
> };
>
> +static int strbuf_set_helper_option(struct helper_data *data,
> + struct strbuf *buf)
> +{
> + int ret;
> +
> + sendline(data, buf);
> + if (recvline(data, buf))
> + exit(128);
> +
> + if (!strcmp(buf->buf, "ok"))
> + ret = 0;
> + else if (starts_with(buf->buf, "error")) {
> + ret = -1;
> + } else if (!strcmp(buf->buf, "unsupported"))
> + ret = 1;
> + else {
> + warning("%s unexpectedly said: '%s'", data->name, buf->buf);
> + ret = 1;
> + }
> + return ret;
> +}
> +
> static int set_helper_option(struct transport *transport,
> const char *name, const char *value)
> {
> @@ -291,20 +313,7 @@ static int set_helper_option(struct transport *transport,
> quote_c_style(value, &buf, NULL, 0);
> strbuf_addch(&buf, '\n');
>
> - sendline(data, &buf);
> - if (recvline(data, &buf))
> - exit(128);
> -
> - if (!strcmp(buf.buf, "ok"))
> - ret = 0;
> - else if (starts_with(buf.buf, "error")) {
> - ret = -1;
> - } else if (!strcmp(buf.buf, "unsupported"))
> - ret = 1;
> - else {
> - warning("%s unexpectedly said: '%s'", data->name, buf.buf);
> - ret = 1;
> - }
> + ret = strbuf_set_helper_option(data, &buf);
> strbuf_release(&buf);
> return ret;
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
