Re: [RFC] remove/deprecate 'submodule init' and 'sync'

From: W. Trevor King <wking@tremily.us>
Date: Dec 2, 2012, 21:11 UTC
Message-ID: <20121202211159.GA12429@odin.tremily.us>
In-Reply-To: <50BBBA29.2000106@web.de>

From: "W. Trevor King" <wking@tremily.us>
To: Jens Lehmann <Jens.Lehmann@web.de>, Junio C Hamano <gitster@pobox.com>
Cc: Phil Hord <phil.hord@gmail.com>, Git <git@vger.kernel.org>,
	Heiko Voigt <hvoigt@hvoigt.net>, Jeff King <peff@peff.net>,
	Shawn Pearce <spearce@spearce.org>, Nahor <nahor.j+gmane@gmail.com>
Bcc: 
Subject: Re: [RFC] remove/deprecate 'submodule init' and 'sync'
Reply-To: 
In-Reply-To: <50BBBA29.2000106@web.de>
 <50BBB22A.7050901@web.de>
 <20121202190929.GG9401@odin.tremily.us>
OpenPGP: id=39A2F3FA2AB17E5D8764F388FC29BDCDF15F5BE8;
 url=http://tremily.us/pubkey.txt

On Sun, Dec 02, 2012 at 09:29:29PM +0100, Jens Lehmann wrote:

Show 15 quoted lines

> Am 02.12.2012 20:09, schrieb W. Trevor King:
> > Before I get into the details, I'd like to point out that I actually
> > understand the purpose of `submodule init` now ;).  To avoid further
> > confusion, my current one-line command summaries would be:
> > 
> >   init:   mark a submodule as active for future submodule operation
> >   deinit: mark a submodule as inactive for future submodule operation
> >   sync:   update remote.<name>.origin in submodules to reflect changes
> >           in .gitmodules or the superproject's remote URL.
> > 
> > I don't think we disagree on that, we just don't agree on how to
> > implement it.
> 
> Nope, it is already implemented and you are arguing to change the
> current implementation.

Agreed.

Show 11 quoted lines

> To quote from another mail:
> 
> Am 01.12.2012 18:49, schrieb W. Trevor King:
> > On Sat, Dec 01, 2012 at 06:25:17PM +0100, Jens Lehmann wrote:
> >> What real world problems do we have with the current init/sync that
> >> this approach would solve?
> >
> > I don't have any, ...
> 
> We don't want to change working code and cause compatibility issues
> just because we /could/ do things differently, no?

In principle, yes, but in this case I think changing the implementation does not risk much in the way of compatibility issues (it only hurts users who rely on `submodule init` setting submodule.<name>.url for reasons of their own. A few of the existing tests explictly check the url setting, so perhaps there are a number of users who do require this side effect?

I think this risk is outweighed by the benefits of having a clearer activation option. For example:

On Sun, Dec 02, 2012 at 08:55:22PM +0100, Jens Lehmann wrote:

Show 5 quoted lines

> Sure. I was worried about throwing away other settings the user
> might have set in the submodule.$name section and the first reflex
> was to protect them. But thinking about that again I noticed we are
> already throwing away a possibly user customized "url" setting, so
> we already remove a possibly customized setting.

With submodule.<name>.active, there's nothing customized that you'd have to nuke on deinit (except 'active' iteself, which the user is explicitly asking for).

Cheers, Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy