git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Philippe Blain <levraiphilippeblain@gmail.com>
To: Abhradeep Chakraborty via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: Philip Oakley <philipoakley@iee.email>,
	Junio C Hamano <gitster@pobox.com>,
	Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
Subject: Re: [PATCH v3] builtin/remote.c: teach `-v` to list filters for promisor remotes
Date: Sun, 8 May 2022 11:44:23 -0400	[thread overview]
Message-ID: <f15e2673-ddc3-27ff-d31c-7fa32af27ae7@gmail.com> (raw)
In-Reply-To: <pull.1227.v3.git.1651933221216.gitgitgadget@gmail.com>

Forgot to comment on the patch itself :P

Le 2022-05-07 à 10:20, Abhradeep Chakraborty via GitGitGadget a écrit :
> From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
> 

>  Documentation/git-remote.txt |  3 +++
>  builtin/remote.c             | 18 +++++++++++++-----
>  t/t5616-partial-clone.sh     | 28 ++++++++++++++++++++++++++++

I think the tests woud fit better in t5505-remote.sh, since the patch really
adds a feature to the 'git remote' command. 

>  3 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
> index cde9614e362..a125bd839f7 100644
> --- a/Documentation/git-remote.txt
> +++ b/Documentation/git-remote.txt
> @@ -35,6 +35,9 @@ OPTIONS
>  -v::
>  --verbose::
>  	Be a little more verbose and show remote url after name.
> +	For promisor remotes it will show an extra information

I found it sligtly awkward to use the future tense here. Maybe just:

    For promisor remotes, also show which filter
    (`blob:none` etc.) that promisor remote use, wrapped in square brackets.

And technically, it's not really the remote that "uses" the filter, 
but more the local Git client. So maybe something like this would
be more accurate and simpler:

    For promisor remotes, also show which filter (`blob:none` etc.)
    are configured, wrapped in square brackets.

And even then "wrapped in square brackets" *could* be dropped, I 
think.

Apart from that, the patch and test look good, thanks for working
on that!

Cheers,
Philippe.

  parent reply	other threads:[~2022-05-08 15:45 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-30 13:19 [PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes Abhradeep Chakraborty via GitGitGadget
2022-04-30 21:17 ` Junio C Hamano
2022-05-01 15:57   ` Abhradeep Chakraborty
2022-05-01 16:14     ` Junio C Hamano
2022-05-01 19:38       ` Abhradeep Chakraborty
2022-05-02 10:33         ` Philip Oakley
2022-05-02 14:56           ` Abhradeep Chakraborty
2022-05-03 15:20 ` [PATCH v2] " Abhradeep Chakraborty via GitGitGadget
2022-05-04 17:10   ` Junio C Hamano
2022-05-05 14:12     ` Abhradeep Chakraborty
2022-05-07 14:20   ` [PATCH v3] " Abhradeep Chakraborty via GitGitGadget
2022-05-08 15:33     ` Philippe Blain
2022-05-09 16:29       ` Junio C Hamano
2022-05-09 16:45         ` Philippe Blain
2022-05-08 15:44     ` Philippe Blain [this message]
2022-05-09  9:13       ` Abhradeep Chakraborty
2022-05-09 11:32     ` [PATCH v4] " Abhradeep Chakraborty via GitGitGadget
2022-05-09 15:34       ` Taylor Blau
2022-05-09 17:01         ` Philippe Blain
2022-05-09 17:52           ` Junio C Hamano
2022-05-13 13:49             ` Abhradeep Chakraborty
2022-05-13 18:37               ` Junio C Hamano
2022-05-16 15:38                 ` Abhradeep Chakraborty
2022-05-09 17:21         ` Abhradeep Chakraborty
2022-05-09 22:22           ` Taylor Blau
2022-05-09 17:44         ` Abhradeep Chakraborty

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=f15e2673-ddc3-27ff-d31c-7fa32af27ae7@gmail.com \
    --to=levraiphilippeblain@gmail.com \
    --cc=chakrabortyabhradeep79@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=philipoakley@iee.email \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).