All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	"git\@vger.kernel.org" <git@vger.kernel.org>,
	Jens Lehmann <Jens.Lehmann@web.de>,
	Duy Nguyen <pclouds@gmail.com>
Subject: Re: [PATCH 11/15] diff: ignore submodules excluded by groups
Date: Thu, 05 May 2016 13:19:22 -0700	[thread overview]
Message-ID: <xmqqy47o9s1h.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAGZ79ka37jWYDJrAWy5KLhaaJmrLRbmTzRC6A5DneuE63+XCeQ@mail.gmail.com> (Stefan Beller's message of "Thu, 5 May 2016 12:57:25 -0700")

Stefan Beller <sbeller@google.com> writes:

> Any thoughts on my thoughts below?
>
>> So here is a thought experiment:
>>
>>     # get all submodules into the work tree
>>     git submodule update --recursive --init
>>
>>     # The selected default group will not include all submodules
>>     git config submodule.defaultGroup "*SomeLabel"
>>
>>     git status
>>     # What do we expect now?
>>     # either a "nothing to commit, working directory clean"
>>     # or rather what was described in 0/15:
>>
>>         More than 2 submodules (123 actually) including
>>             'path/foo'
>>             'lib/baz'
>>         are checked out, but not part of the default group.
>>         You can add these submodules via
>>             git config --add submodule.defaultGroup ./path/foo
>>             git config --add submodule.defaultGroup ./lib/baz

That may be an interesting thing to know, but I am not sure if it
adds value to the user.  The user wanted the defaultGroup to be
the set of submodules labeled with SomeLabel, and an alternative
valid suggestion could be

	'path/foo' and other submodules are not part of what you are
	interested in; if you want to deinitialize them, do

            git submodule deinit !defaultGroup

Both look equally valid to me, but offering both would be way too
much.  I'd say you should give that only with "status -v" or
something, perhaps?

>> If we want to go with the second option,

You already lost me here, as it is not clear what two "options" you
are comparing.

>> If we want to go with the second option, the design described in 0/15
>> is broken. Going one step further:
>>
>> ...
>>     # But what about subC which is not in the default group? It was
>> changed as well.

So why not show it?  Is there anything controversial?

If you are truly not interested in it by excluding it from the
default group, you wouldn't have touched it in the first place.  If
you did touch it, then you are showing some special interest that
overrides what you said in the default mechanism.

In short, I think I understood what you wanted with your analogy to
the ignore/exclude mechanism in 0/15.  Default is a handy way to say
"I do not want to bother specifying everything from the command line
every time" but we can say that it is nothing more than that.  That
is exactly how the ignore/exclude mechanism is used--"git add" by
default will not add those that are ignored when discovering paths
by recursively descending, but once added, that is part of what the
user told Git that she cares about.

>> In case we report these submodules which are checked out but not in
>> the default group, we probably want to adapt "git submodule update" to
>> un-checkout the work tree of the submodules in case they are clean.

Why?

Letting them know that they have such an option, and giving them a
way to tell which submodules fall into that category, are both good
things.  But why is it a good thing to automatically clean what the
user has checked out (which I expect that she expects to remain
until she explicitly tells us otherwise)?

We do not automatically "git rm" a clean tracked path that happens
to match .gitignore pattern and I do not think it is a good thing to
do so.

Puzzled.

  reply	other threads:[~2016-05-05 20:19 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-26 20:50 [PATCH 00/15] submodule groups (once again) Stefan Beller
2016-04-26 20:50 ` [PATCH 01/15] string_list: add string_list_duplicate Stefan Beller
2016-04-26 22:37   ` Junio C Hamano
2016-04-27 18:02     ` Stefan Beller
2016-04-27 21:11       ` Junio C Hamano
2016-04-27 21:17         ` Stefan Beller
2016-04-27 23:17           ` Junio C Hamano
2016-04-27 23:24             ` Stefan Beller
2016-04-26 20:50 ` [PATCH 02/15] submodule doc: write down what we want to achieve in this series Stefan Beller
2016-04-26 22:42   ` Junio C Hamano
2016-04-26 20:50 ` [PATCH 03/15] submodule add: label submodules if asked to Stefan Beller
2016-04-26 22:49   ` Junio C Hamano
2016-04-26 22:50   ` Jacob Keller
2016-04-26 23:19     ` Stefan Beller
2016-04-27  3:24       ` Jacob Keller
2016-04-26 20:50 ` [PATCH 04/15] submodule-config: keep labels around Stefan Beller
2016-04-26 20:50 ` [PATCH 05/15] submodule-config: check if submodule a submodule is in a group Stefan Beller
2016-04-26 22:58   ` Junio C Hamano
2016-04-26 23:17     ` Junio C Hamano
2016-04-27 23:00       ` Stefan Beller
2016-04-26 20:50 ` [PATCH 06/15] submodule init: redirect stdout to stderr Stefan Beller
2016-04-29 18:27   ` Junio C Hamano
2016-04-29 18:38     ` Stefan Beller
2016-04-26 20:50 ` [PATCH 07/15] submodule deinit: loose requirement for giving '.' Stefan Beller
2016-04-29 18:27   ` Junio C Hamano
2016-04-26 20:50 ` [PATCH 08/15] submodule--helper list: respect submodule groups Stefan Beller
2016-04-29 18:31   ` Junio C Hamano
2016-04-26 20:50 ` [PATCH 09/15] submodule--helper init: " Stefan Beller
2016-04-26 20:50 ` [PATCH 10/15] submodule--helper update_clone: " Stefan Beller
2016-04-26 20:50 ` [PATCH 11/15] diff: ignore submodules excluded by groups Stefan Beller
2016-04-29 18:37   ` Junio C Hamano
2016-04-29 19:17     ` Stefan Beller
2016-05-05 19:57       ` Stefan Beller
2016-05-05 20:19         ` Junio C Hamano [this message]
2016-05-05 21:02           ` Stefan Beller
2016-05-05 22:20             ` Junio C Hamano
2016-05-05 22:50               ` Stefan Beller
2016-05-05 23:07                 ` Junio C Hamano
2016-05-06  6:08                   ` Junio C Hamano
2016-05-06 18:23                     ` Stefan Beller
2016-05-06 18:59                       ` Junio C Hamano
2016-04-26 20:50 ` [PATCH 12/15] git submodule summary respects groups Stefan Beller
2016-04-29 18:38   ` Junio C Hamano
2016-04-26 20:50 ` [PATCH 13/15] cmd_status: respect submodule groups Stefan Beller
2016-04-26 20:50 ` [PATCH 14/15] cmd_diff: " Stefan Beller
2016-04-26 20:50 ` [PATCH 15/15] clone: allow specification of submodules to be cloned Stefan Beller
2016-04-26 22:19 ` [PATCH 00/15] submodule groups (once again) Junio C Hamano
2016-04-26 22:26 ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=xmqqy47o9s1h.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=pclouds@gmail.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.