Subject: Re: [PATCH 17/18] revert: Don't implictly stomp pending sequencer operation

From: Jonathan Nieder <[email-masked]>
Date: Wed, 27 Jul 2011 16:33:46 +0200

Ramkumar Ramachandra wrote:
> Jonathan Nieder writes:
>> Ramkumar Ramachandra wrote:

>>> +             if (create_seq_dir() < 0) {
>>> +                     advise(_("A cherry-pick or revert is in progress."));
>>> +                     advise(_("Use --reset to forget about it"));
>>> +                     return -1;
>>> +             }
>>
>> The usual formula is:
>>
>>        error(... description of error ...)
>>        if (advice_foo_bar [i.e., if the user is not tired of the advice already]) {
>>                advise(... how to recover from error ...);
>>                advise(... more lines ...);
>>        }
>
> I think you're trying to say two things here:
> 1. Put the error() call in the caller.

No, I didn't mean anything about the caller.  Here the error is "a
cherry-pick or revert is in progress".  Before the advice mechanism
existed, such a line would have been written with error(), not note(),
right?

[...]
> 2. Guard the advice using a variable.  I have to invent a new
> configuration variable; can't that wait*?

Sure, if the advice is quiet (as it is in this case), I suppose there
is no need to guard it until someone using it complains. :)

I mentioned advice configuration to emphasize that output from running
git with all "hint:" lines stripped out is supposed to be just as
sensible as with them, if I understand the purpose of the facility
correctly.

Thanks for thinking it over.