Subject: Re: [PATCH 6/7] git rm: Support for removing submodules

From: Petr Baudis <[email-masked]>
Date: Thu, 17 Jul 2008 14:35:01 +0200

Hi,

On Thu, Jul 17, 2008 at 12:41:57AM +0200, Johannes Schindelin wrote:
> On Wed, 16 Jul 2008, Petr Baudis wrote:
> 
> > This patch adds support for removing submodules to 'git rm', including 
> > removing the appropriate sections from the .gitmodules file to reflect 
> > this
> 
> I have no time to look at the whole series, or even at the patch, but I 
> have concerns.  Do you really remove the whole directory?  Because if you 
> do, you remove more than what can be possibly reconstructed from the 
> object store.

  no; I remove only the index entry and the empty directory made for
non-checked-out submodules (I just try rmdir() and ignore ENOTEMPTY
error).  I make that clear in git rm documentation, but not in the patch
description; I will fix that.

> I wonder if it really makes sense to integrate that into git-rm, and not 
> git-submodule, if only to introduce another level of consideration for the 
> user before committing what is potentially a big mistake.

  That is good question and I forgot to elaborate on this in the cover
letter. However, I believe that integrating this functionality into the
basic commands makes for a smoother user experience, following the
principle of the least surprise. It is difficult for me to argue
extensively in further favor of this choice, though. :-)

-- 
				Petr "Pasky" Baudis
GNU, n. An animal of South Africa, which in its domesticated state
resembles a horse, a buffalo and a stag. In its wild condition it is
something like a thunderbolt, an earthquake and a cyclone. -- A. Pierce