Re: [PATCH] push: Provide situational hints for non-fast-forward errors

[Christopher Tiwald <christiwald@gmail.com>]

On Wed, Mar 14, 2012 at 03:27:52PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> On Wed, Mar 14, 2012 at 02:00:38PM +0100, Matthieu Moy wrote:
> > Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> > 
> > > I think that having three different config keys for the three
> > > different advices makes sense, because the advices will be displayed
> > > at different times.
> > 
> > I don't think it really makes sense to be such fine-grained. We already
> > have 6 different advices, so an advance user who do not want them need
> > to set these 6 variables. I think we want to keep this number relatively
> > low.
> > 
> > The advice messages do not point explicitely to the way to disable them,
> > so users who know how to set advice.* are users who know a little about
> > configuration files, and who read the docs. 
> 
> Elsewhere in this thread it was proposed to add an actual 'git config'
> command to the advice.

After considering it, I tend to agree that three different config keys
is overkill. I feel like users who disable advice are doing it because
they find the messages annoying, not because they've mastered that
particular situation and no longer need the reminder. Forcing them to
disable three different options to get an advice-less 'git push' seems
like it'd just be irritating.

I could be wrong about that. Perhaps users who graduate workflows as you
described above are more common? I don't disable any advice locally and
thus can't speak well to what motivates that decision.
> 
> > Instead of having too
> > fine-grained configuration variables, we can have a better doc,
> > explaining shortly the 3 possible cases under advice.nonfastforward in
> > config.txt. The user who disable the advice can read the doc (I usually
> > think that "users don't read documentation" is a better assumption, but
> > since the user knows about the name of the variable, it is OK here).
> > 
> > Also, if I read correctly the patch, the old variable is left in the doc
> > and in advice.{c,h}, but is no longer used. This means old-timers who
> > have set it will see the message poping-up again after they upgrade,
> > which I think is inconveinient for them.
> 
> So it seems that the old variable should be respected, not to annoy
> "old-timers".

I hadn't considered users who already have the variable set. I'll
correct for that. I'll also attempt to improve the doc for
advice.nonfastforward.

--
Christopher Tiwald