Re: [PATCH 2/2] checkout: do not mention detach advice for explicit --detach option

From: Stefan Beller <sbeller@google.com>
Date: Aug 15, 2016, 18:54 UTC
Message-ID: <CAGZ79kZq=dPsngaqAVOj6UhaJE0=eR==XSTSSQCmcUTKugeM0Q@mail.gmail.com>
In-Reply-To: <20160815184730.eevqogqrxp2zp43q@sigill.intra.peff.net>

On Mon, Aug 15, 2016 at 11:47 AM, Jeff King <peff@peff.net> wrote:

Show 18 quoted lines

> On Mon, Aug 15, 2016 at 11:40:21AM -0700, Stefan Beller wrote:
>
>> When a user asked for a detached HEAD specifically with `--detach`,
>> we do not need to give advice on what a detached HEAD state entails as
>> we can assume they know what they're getting into as they asked for it.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>
>>  Junio writes:
>>  > It might be controversial how the second from the last case should
>>  > behave, though.
>>
>>  I agree. I think if the advice is configured explicitly we can still give it.
>>  That makes the code a bit more complicated though.
>
> So....I guess. But has anybody in the history of git ever explicitly
> configured advice.* to true?

An admin/teacher of a university that wants to teach Git to students in a class? Better be safe than sorry and explicitly ask for advice because... we cannot trust the default?

>
> It has never produced any change of behavior, and the whole point of
> "advice.*" was that git would advise by default, and you would use
> advice.* to shut it up once you were sufficiently educated.

And now I am arguing that "by default" we should not give advice 100% of the time, but only when we think it is appropriate. You may disagree (as a teacher see above), so you can slightly change the setting to give out advice more often again?

Show 9 quoted lines

>
> I don't think doing it this way is _wrong_. It just feels sort of
> pointlessly over-engineered. It's also a little weird that all of the:
>
>   if (advice_foo)
>
> will trigger because "advice_foo" is set to -1. I think it does the
> right thing, but it feels like a bug (the value is now a tri-state, and
> we silently collapse two states into one).

I think this is what I did in some of the submodule code as well, which is why I assumed it's ok (or rather the projects groupthink on how to do "default on but still different than explicit on")

If you think this is wrong, what is the idiomatic way to solve this problem?

Thanks, Stefan

>
> -Peff