Re: [PATCH v2 1/1] commit: display advice hints when commit fails

[Junio C Hamano <gitster@pobox.com>]

Jonathan Tan <jonathantanmy@google.com> writes:

>> In any case, here is what I tentatively have in my tree (with heavy
>> rewrite to the proposed log message).
>
> Junio, what are your plans over what you have in your tree? If you'd
> like to hear Heba's opinion on it, then she can chime in; if you'd like
> a review, then I think it's good to go in.

On hold until anything like those happens ;-) 

A random reviewer mentioning something on a patch (either in a
line-by-line critique form or "how about doing it this way instead"
counterproposal form) without getting followed up by others
(including the original author) is a stall review thread, and it
does not change the equation if the random reviewer happens to be me.

>> I didn't try it on my end. Maybe it won't help much, because we think
>> we're going to use the editor right up until we realize it's not
>> committable?
>
> And I think the answer to that is "s" is used throughout the function in
> various ways (in particular, used to print statuses both to stdout and
> to the message template) so any wrapping or corralling of scope would
> just make things more complicated. In particular, the way Heba did it in
> v2 is more unclear - at the time of setting s->hints = 0, it's done

You mean "less clear" (just double checking if I got the negation right)?

> within a "if (use_editor && include_status)" block, but (as far as I can
> tell) the commit message template might also be used when there is no
> editor - for example, as input to a hook. And more importantly, when
> s->hints is reset to the config, we don't know at that point that the
> next status is going to stdout. So I think it's better just to use the
> v1 way.

Yeah, thanks for going back to compare v1 and v2, and I agree with
your assessment.

> The second area of discussion I see is in the commit message. Commit
> messages have to balance brevity and comprehensiveness, and this can be
> a subjective matter, but I think Junio's strikes a good balance.

As one side of the comparison is my own, I won't be a good judge on
this, but yes I tried to strick a good balance as much as possible.

I think I've merged it to 'next' yesterday, but it does not mean
that much as we are in -rc and it is not such an urgent "oops we
broke it in this cycle, let's fix it" issue.  If we see a v3 that
improves it, I do not mind at all reverting what I merged to 'next'
and use the updated one instead (either way, it will be in 'master'
during the next cycle at the earliest).

Thanks.