Re: [PATCH 01/14] advice: Introduce error_resolve_conflict

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 6, 2011, 09:28 UTC
Message-ID: <CALkWK0k57L5vc7oVQKw=G9FXtYkmnp8QaWrW_p1CvupyNGofZg@mail.gmail.com>
In-Reply-To: <20110706083512.GA15682@elie>

Hi Jonathan,

Jonathan Nieder writes:

Show 19 quoted lines

> Ramkumar Ramachandra wrote:
>> --- a/advice.c
>> +++ b/advice.c
>> @@ -19,6 +19,15 @@ static struct {
>>       { "detachedhead", &advice_detached_head },
>>  };
>>
>> +static void advise(const char *advice, ...)
>> +{
>> +     va_list params;
>> +
>> +     va_start(params, advice);
>> +     vreportf("hint: ", advice, params);
>> +     va_end(params);
>> +}
>
> Rather than copy+pasting this code verbatim, wouldn't it make sense to
> move it and expose it through advice.h so the old call site can use
> the same code?

Yes, but I was worried that I shouldn't expose it because your commit message (2a41df) says:

    It is local to revert.c for now because I am not sure this is
    the right API (we may want to take an array of advice lines or a
    boolean argument for easy suppression of unwanted advice).

So, is it still alright to expose it in advice.h?

> For what it's worth, with that change,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

-- Ram