git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>,
	Git <git@vger.kernel.org>
Subject: Re: [PATCH] builtin/remote.c: teach `-v` to list filters for promisor remotes
Date: Mon,  2 May 2022 01:08:07 +0530	[thread overview]
Message-ID: <20220501193807.94369-1-chakrabortyabhradeep79@gmail.com> (raw)
In-Reply-To: <xmqqfslt44di.fsf@gitster.g>


Junio C Hamano <gitster@pobox.com> wrote:

> You forgot to answer more important half of the question.  It would
> be easy for you to know what the string inside brackets means
> because you are so obsessed with the promisor remote to write this
> patch ;-) But when we need to add even more pieces of information in
> the future, will it stay so?  Can "[some-random-string]" easily be
> identified as a list-objects-filter by those who do not care
> particularly about promisor remotes (e.g. those who wanted to see
> the URL to tell multiple remote nicknames apart) when the line has
> even more piece of information in the future?
>
> At some point, we'd need to either (1) stop adding too many details
> to avoid cluttering the output line, or (2) start labeling each
> piece of information to make it easy for the readers to identify
> which one is which [*].  We need to ask ourselves why now is not
> that "some point" already.
>
>     Side note: and the strategy to add new pieces of information
>     need to take the same approach between the two, and that is why
>     we need "what is the plan to add new pieces of information?"
>     answered.

I am sorry if I failed to explain you what I really wanted to mean.
Yes, I forgot to answer the last question which is "When we
want to add new kinds of information other than the URL and the
list-objects-filter, what is our plan to add them?".

So let me answer this now. As `-v` flag gives a kind of overall summary
of the remotes, users expect that the most important and most basic
information should be listed in the output of `remote -v`. So, there
may be some other more important informations in the future that we
have to add to `remote -v` output. In that case, method (1) would not
be a great idea I think (unless a new flag has been created). method
(2) is better.

> > (i.e. which are promisor remotes and which are not) one by one. If the
> > user try to search for the promisor remotes in the config file, he/she
> > have to go through the other configuration settings (irrelevant to him/her
> > at that time) to reach the `[remote]` section. Isn't it?
>
> Sorry, but the question does not make much sense to me.  Why is a
> piece of information you get from "git config" irrelevant if you get
> it in order to figure out what you want to know, i.e.  what promisor
> remote do we rely on?

Let me explain what I really meant here - I am guessing that you have no
problem with the upper part of that para.

If we forget about my proposed change, there are two possible ways to find
out the info about promisor remotes - 
	(1) Use `git config --get remote.<remote-name>.partialCloneFilter`

	   This command gives an output only if <remote-name> is a promisor
	   remote. So in case the user forget which one is a promisor
	   remote, he/she has to try this command with each and every
	   <remote-name> to find out which is/are the promisor remote(s).

	(2) Open the git config file (either manually or by running `git
	    config --edit`

	    In this case, the user has to go through all the settings until
	    the [remote "<remote-name>"] section is found. E.g. let's say
	    below is the config file - 

	    [core]
        	repositoryformatversion = 0
        	filemode = true
        	bare = false
        	logallrefupdates = true
        	ignorecase = true
        	precomposeunicode = true
	    [remote "upstream"]
        	url = https://github.com/git/git.git
        	fetch = +refs/heads/*:refs/remotes/upstream/*
	    [branch "master"]
        	remote = upstream
        	merge = refs/heads/master
	    [remote "origin"]
        	url = https://github.com/Abhra303/git.git
        	fetch = +refs/heads/*:refs/remotes/origin/*
		partialCloneFilter = blob:none

	    To find out whether "origin" is promisor or not, he has to go
	    to the [remote "origin"] section. Here all the configuations
	    under `[core]`, `[remote "upstream"]` and `[branch "master"]
	    are irrelevant to him/her at that time (because he/she is not
	    interested to know about those configuration settings at that
	    time).

The proposed change is simpler compared to the above as it lists down all
the remotes along with their list-objects-filter. Another point is that
it's important for an user to know which one is a promisor remote and what
filter type they use. If we go with the current implementation the output
would be let's say - 
origin <remote-url> (fetch)
origin <remote-url> (push)
upstream <remote-url> (fetch)
upstream <remote-url> (push)

By seeing the above output anyone may assume that all the remotes are
normal remotes. If the user now try to run `git pull origin` and suddenly
he/she discover that some blobs are not downloaded. He/she run the above
mentioned (1) command and find that this is a promisor remote!

Here `remote -v` didn't warn the user about the origin remote being an
promisor remote. Instead it makes him/her assume that all are normal
remotes. Providing only these three info (i.e. <remote-name>, <remote-url>
and <direction>) is not sufficient - it only shows the half of the picture.


> The question is "what can readers achieve by having this extra
> information in 'remote -v' output".  Do you have to duck the
> question because you cannot answer in a simple sentence, and instead
> readers must read reams of documentation pages?  I doubt it would be
> that obscure.

Sorry, I misunderstood that section of your first comment. I think
I hopefully answered this question in the above portion of this comment.
Providing only the three information about remotes is not sufficient
as it do not distinguish between promisor remotes and normal remotes.
In that sense, it will add simplicity and the user would be much more
clear about the remotes(i.e. which is promisor remote and which is not).

> I wanted to like the patch, the changed text is simple enough, but
> quite honestly, the lack of clarity in the answers to the most basic
> "why do we want this? what is this good for? how does this help the
> users?" questions, I am not yet succeeding to do so.

My bad! Hope I am now able to answer all the questions you asked. Let
me know if you still struggle to get my point.

Thanks :)

  reply	other threads:[~2022-05-01 19:38 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 [this message]
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
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=20220501193807.94369-1-chakrabortyabhradeep79@gmail.com \
    --to=chakrabortyabhradeep79@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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).