Re: [PATCH 2/2] add: respect add.updateroot config option

[Jeff King <peff@peff.net>]

On Wed, Mar 13, 2013 at 10:07:34AM +0100, Matthieu Moy wrote:

> Jeff King <peff@peff.net> writes:
> 
> > For users who have read and accepted the warning, there is no way to
> > jump directly to the future behavior and silence the warning.
> 
> I think the idea makes sense. The transition period is necessary for
> people who use different versions of Git (which includes anybody
> writting and distributing scripts), but for poor mortals who only use a
> single version of Git, it's nice to be able to jump to the future
> behavior once for all as soon as possible.

I think the biggest risk is from people who think they are safe to jump,
and then find out that some script they depend on is not ready. Even if
they do not even realize they are relying on it. Part of the point of
the transition period is to get script authors to update their scripts,
and to let the new versions trickle down to the users.

> Your patch doesn't advertise the option in the warning message, which I
> think is good. You may mention it the commit message that this is a
> deliberate choice.

Yes, it was deliberate. I can add a note.

> > +add.updateroot::
> 
> Detail: option names are normally camelCased => updateRoot.

Good point, thanks.

> I think the option name needs a bit more thinking. Without reading the
> doc,
> 
> [add]
> 	updateRoot = false
> 
> would be a very tempting thing to try. Perhaps explicitly warning when
> add.updateroot is set to false would avoid possible confusion.

Yeah, that occurred to me, too, hence the note in the doc. Since it
isn't advertised elsewhere, I had hoped that anybody who discovered it
would see the note. I suppose we can warn when we see add.updateRoot set
to anything but true. That feels a bit hacky, as it's possible the user
could be overriding an earlier setting (although that is getting kind of
crazy).

> I liked David's suggestion of using future.* to mean "start using
> behavior from the future right now".

I do like that idea, as it makes the meaning of the variable more clear.

I dunno. I am not all that excited about this patch, due to all of the
caveats that we need to communicate to the user. The current warning has
annoyed me a few times, but perhaps it is still too soon, and my fingers
and brain just need retraining. I think a config option could help some
people, but maybe it will end up hurting more.  I'm inclined at this
point to table the patch for now and see how I feel in a few weeks.

I do think patch 1/2 makes sense regardless.

-Peff