From: Jens Lehmann <Jens.Lehmann@web.de>
To: Jeff King <peff@peff.net>
Cc: Alex Linden Levy <lindenle@gmail.com>,
gitster@pobox.com, git@vger.kernel.org,
Heiko Voigt <hvoigt@hvoigt.net>
Subject: Re: [PATCH] Add rm to submodule command.
Date: Sun, 04 Nov 2012 15:29:02 +0100 [thread overview]
Message-ID: <50967BAE.4030102@web.de> (raw)
In-Reply-To: <20121104134324.GA31623@sigill.intra.peff.net>
Am 04.11.2012 14:43, schrieb Jeff King:
> On Fri, Nov 02, 2012 at 10:26:11AM -0700, Alex Linden Levy wrote:
>
>> This change removes the config entries in .gitmodules and adds it.
>> ---
>
> Signoff?
>
>> git-submodule.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> No documentation or tests?
>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index ab6b110..29d950f 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>
> I'd defer to submodule experts on whether the steps to 'rm' the
> submodule make sense. Jens?
Hmm, this change adds the --quiet and --branch options to rm
which aren't used (and at least --branch makes no sense to me
here). Remainders of copy & paste? It also only affects the
.gitmodules setting and leaves the submodules work tree alone,
while I think it should - at least optionally - remove the work
tree just like "git rm" removes files too (of course only if
there is no .git directory found in it and no modifications are
present, as that would possibly lose data).
Me also thinks such a command should use my recent rm submodule
work to remove the work tree (found in your current master and
Junio's next branch), which does all necessary checks before it
removes the work tree together with the index entry. This could
be tweaked via a --cached option or such if the user wants to
keep the work tree.
But apart from those issues I'm not convinced that adding a
"git submodule rm" command is the best option. While working on
teaching "git mv" to move submodules I came to the conclusion
that it might be a better solution to let "git rm" remove the
submodule entry from the .gitmodules file itself (but of course
only if that file is found and contains such an entry, if not
it will silently do nothing to not disturb submodule users who
don't have a .gitmodules file and are using plain gitlinks).
The reason is that git core commands like status, diff and
fetch already use the path -> name mapping found in .gitmodules
and will behave strange when this is out of sync with the work
tree. So I strongly believe that doing a "git mv" should change
the path -> name mapping in .gitmodules too while moving the
submodule's work tree and updating the index (of course again
only if .gitmodules is found and contains such an entry, if not
it'll just move the work tree and update the index). Then we
won't need a new "git submodule mv" command as everything is
done inside the mv command. And for consistency I think "git rm"
should also remove the path -> name mapping (even though that is
not required for a rm to do its job, as no one will use that
setting later when the submodule is gone from the index). Then
we won't need a new "git submodule rm" at all.
Does that make sense?
prev parent reply other threads:[~2012-11-04 14:30 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-02 17:26 [PATCH] Add rm to submodule command Alex Linden Levy
2012-11-04 13:43 ` Jeff King
2012-11-04 14:29 ` Jens Lehmann [this message]
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=50967BAE.4030102@web.de \
--to=jens.lehmann@web.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=hvoigt@hvoigt.net \
--cc=lindenle@gmail.com \
--cc=peff@peff.net \
/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 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 NNTP newsgroup(s).