Re: [PATCH v3] builtin/remote.c: teach `-v` to list filters for promisor remotes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Forgot to comment on the patch itself :P

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

>  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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux