git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: "git\@vger.kernel.org" <git@vger.kernel.org>,
	Jens Lehmann <Jens.Lehmann@web.de>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCHv2 0/6] git clone: Marry --recursive and --reference
Date: Tue, 09 Aug 2016 11:44:36 -0700	[thread overview]
Message-ID: <xmqq8tw5aijv.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAGZ79kY8EiGaugsh4FxKYp1FxqYr10JfGqsrfsnhULB+OBnFXw@mail.gmail.com> (Stefan Beller's message of "Tue, 9 Aug 2016 11:09:34 -0700")

Stefan Beller <sbeller@google.com> writes:

> The way I understood and implemented it is
>
>     here is a path, try to use it as an alternate; if that is not
>     an alternate, it's fine too; maybe warn about it, but carry
>     on with the operation.

My expectation is without "maybe warn about it".  And not adding it
as an alternate (because it is not usable) is just "doing as I was
told to do", nothing noteworthy.  Because "do it only if you can" is
an explicit instruction to the doer that the caller does not care
about the outcome, I'd think there shouldn't be any warning, whether
it is used or not.  At the same time, if the caller wants to know
the outcome, and using if-able as a way to say "I prefer to see it
happen, but you do not have to panic if you can't", I'd think it is
OK to give "info:" to report which of the two possible paths was
taken in either case.  Throwing a "warning:" only when it didn't do
so does not make much sense to me.

> The way you write this response I think you have a slightly
> different understanding of what the -if-able ought to do?
>
>     When --reference is given, only the superproject should
>     borrow and the -if-able, may allow submodules to also borrow?

I have no idea what this sentence means.

>> I have a very strong opinion what should happen when the end-user
>> facing command is "git submodule", but if I have to choose, I would
>> prefer to see "git submodule update" tell what it did with "info:"
>> either case, i.e. "info: borrowing from ... because the superproject
>> borrows from there", and "info: not borrowing from ... even though
>> the superproject borrows from there".
>
> I see. This way the user is most informed.

Yes, and if we were to go that route, "clone" without "-v" should
not report which of the two it took.  It is OK for "submodule" to
look at what "clone" left as the result and do more intelligent
reporting that is better suited for the context of the command.

> The current implementation
> of "submodule update --init" for start from scratch yields for example:
>
> Submodule 'foo' (<url>) registered for path 'foo'
> Submodule 'hooks' (<url>) registered for path 'hooks'
> Cloning into '/home/sbeller/super/foo'...
> Cloning into '/home/sbeller/super/hooks'...
> warning: Not using proposed alternate
> /home/sbeller/super-reference/.git/modules/hooks/
> Submodule path 'foo': checked out '7b41f3a413b46140b050ae5324cbbcdd467d2b3a'
> Submodule path 'hooks': checked out '3acc14d10d26678eae6489038fe0d4dad644a9b4'
>
> so before this series we had 3 lines per submodule, and with this series
> we get a 4th line for the unusual case.
>
> You would prefer to see always 4 lines per submodule?

If the user says "--recursive --reference", then the user probably
deserves to be notified.  As I said (eh, rather, as I wanted to say
but failed to say so), I do not have a strong preference either way.


  reply	other threads:[~2016-08-09 18:44 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-09  4:08 [PATCHv2 0/6] git clone: Marry --recursive and --reference Stefan Beller
2016-08-09  4:08 ` [PATCHv3 1/9] t7408: modernize style Stefan Beller
2016-08-09  6:59   ` Eric Sunshine
2016-08-09 15:50     ` Junio C Hamano
2016-08-09 15:51       ` Junio C Hamano
2016-08-09 17:30         ` Stefan Beller
2016-08-09 17:39           ` Junio C Hamano
2016-08-09 23:06             ` Eric Sunshine
2016-08-09  4:08 ` [PATCHv3 2/9] t7408: merge short tests, factor out testing method Stefan Beller
2016-08-09 16:41   ` Junio C Hamano
2016-08-09 17:26     ` Stefan Beller
2016-08-09  4:08 ` [PATCHv3 3/9] submodule--helper module-clone: allow multiple references Stefan Beller
2016-08-09  4:08 ` [PATCHv3 4/9] submodule--helper update-clone: " Stefan Beller
2016-08-09  4:08 ` [PATCHv3 5/9] clone: clarify option_reference as required Stefan Beller
2016-08-09  4:08 ` [PATCHv2 5/6] submodule update: add super-reference flag Stefan Beller
2016-08-09  4:08 ` [PATCHv3 6/9] clone: implement optional references Stefan Beller
2016-08-09 16:37   ` Junio C Hamano
2016-08-09 17:54     ` Junio C Hamano
2016-08-09 18:20       ` Stefan Beller
2016-08-09 18:35         ` Junio C Hamano
2016-08-09  4:08 ` [PATCHv2 6/6] clone: reference flag is used for submodules as well Stefan Beller
2016-08-09  4:08 ` [PATCHv3 7/9] submodule helper: pass through --reference-if-able Stefan Beller
2016-08-09  4:08 ` [PATCHv3 8/9] submodule: try alternates when superproject has an alternate Stefan Beller
2016-08-09  4:08 ` [PATCHv3 9/9] submodule--helper: use parallel processor correctly Stefan Beller
2016-08-09 16:40   ` Junio C Hamano
2016-08-09  5:23 ` [PATCHv2 0/6] git clone: Marry --recursive and --reference Jacob Keller
2016-08-09 15:49 ` Junio C Hamano
2016-08-09 17:26   ` Stefan Beller
2016-08-09 17:47     ` Junio C Hamano
2016-08-09 17:58       ` Junio C Hamano
2016-08-09 18:09       ` Stefan Beller
2016-08-09 18:44         ` Junio C Hamano [this message]
2016-08-09 20:31           ` Stefan Beller
2016-08-09 21:45             ` Junio C Hamano
2016-08-09 22:05               ` Stefan Beller
2016-08-10 15:59                 ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2016-08-06  1:23 Stefan Beller
2016-08-06 17:29 ` Junio C Hamano
2016-08-08 18:16   ` Stefan Beller

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=xmqq8tw5aijv.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=sbeller@google.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).