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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 31, 2019, 19:06 UTC
Message-ID: <xmqq7e2cfh68.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20191231000420.32396-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 6 quoted lines

>> 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.

Show 9 quoted lines

>> 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)?

Show 6 quoted lines

> 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.