Re: [PATCH] commit & merge: modularize the empty message validator

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2017-07-11 at 13:22 -0700, Junio C Hamano wrote:
> I think the "validation" done with the rest_is_empty() is somewhat
> bogus.  Why should we reject a commit without a message and a
> trailer block with only signed-off-by lines, while accepting a
> commit without a message and a trailer block as long as the trailer
> block has something equally meaningless by itself, like
> "Helped-by:"?  I think we should inspect the proposed commit log
> message taken from the editor, find its tail ignoring the trailing
> comment using ignore_non_trailer, and further separate the result
> into (<message>, <trailers>, <junk at the tail>) using the same
> logic used by the interpret-trailers tool, and then complain when
> <message> turns out to be empty, to be truly useful and consistent.
> 
I have a few doubts for which I need clarification to move on with
this. 

    1. If we abort when the <message> part is empty wouldn't it be too
    restrictive ?

    IOW, Wouldn't it affect users of "git commit -‍-cleanup=verbatim"
    who wish to commit only the comments or parts of it ?
    (I'm not sure if someone would find that useful)

    2. Is it ok to use the "find_trailer_start" function of "trailer.c"
    to locate the trailer? 

    Note: It has a little issue that it wouldn't detect the trailer if
    the message comprises of one trailer alone and no other text. This
    case occurs while aborting a commit started using "git commit -s".
    Any possibilities to overcome the issue?

    3. Ignoring point 1 for now, What other helper methods except the
    ones listed below could be helpful in the separating the cleaned up
    commit message into the <message>, <trailer>, <junk-at-tail> ?

        * ignore_non_trailer
        * find_trailer_start

-- 
Kaartic



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux