All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, me@ikke.info
Subject: Re: [PATCH] commit & merge: modularize the empty message validator
Date: Fri, 14 Jul 2017 23:19:12 +0530	[thread overview]
Message-ID: <1500054552.18990.8.camel@gmail.com> (raw)
In-Reply-To: <xmqqiniww37i.fsf@gitster.mtv.corp.google.com>

On Thu, 2017-07-13 at 12:23 -0700, Junio C Hamano wrote:
> All good points; if it bothers you that "commit" and "merge" define
> "emptyness" of the buffer differently too much, I think you could
> persuade me to unify them to "the buffer _must_ contain no bytes",
> i.e. not special-casing sign-off lines only "commit".
> 
Intereseting, let me give it a try.

To persuade you with this, I have to convince you that the current
behaviour (special-casing of sign-off lines) is defective and/or
biased. It really is for quite a few reasons,

            * Though it's not apparent, it indirectly seems to be hindering
            (to some extent) the idea of including the sign-off (or) other
            trailers which *can't be modified* by the user.

            IOW, the current behaviour seems make the contributors/users
            falsely believe (at least to some extent) that git *does* have
            trailers for commit messages and thus preventing them from coming
            up with ideas that could make "untouchable trailers" a reality.

            Thus, consider "the buffer _must_ contain no bytes" hoping this
            would initiate a "Butterfly effect" :)


        * Looking from an implementation perspective, it's biased in that
        it checks only for sign-offs. Making it work in general is
        difficult as there's no standard definition for the term
        <trailer>. That's because it varies with respect to usage, I
        think. Different people/projects may consider different lines to
        be trailer lines. A few examples are,

            * Bug:
            * Fixes:
            * Change-id:
            * Helped-by:

        Moreover, some people may wish to have commit messages that only
        have such trailers (e.g. "Fixes:"). So, it's difficult to do a
        generalized implementation that aborts when the message is empty
        or consists only of trailers.

        Thus, consider "the buffer _must_ contain no bytes" because it's
        not easy to define what a <trailer> means and special casing
        "sign-off" is biased.


        * Imagine a hypothetical version of git that aborts when the
        <message> is empty though a <trailer> is present. This would
        quite possibly instigate controversies as the "hypothetical git"
        reduces the "valid commit messages" and would quite possibly
        reject a commit message as "empty" (which is uncommunicative)
        though a previous version (which did not have this change)
        accepted a similar message.

        SO, bringing in the Occam's razor, let's choose the option that's
        the simplest and makes the fewest assumptions.


Thus, I conclude that the considering a commit message consisting only
of <trailer>s as empty isn't a good idea and should be dropped.


-- 
Kaartic

  reply	other threads:[~2017-07-14 17:54 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-02 12:03 Why doesn't merge fail if message has only sign-off? Kaartic Sivaraam
2017-07-03 17:21 ` Junio C Hamano
2017-07-04 20:03   ` Kaartic Sivaraam
2017-07-06  3:31   ` [PATCH] merge-message: change meaning of "empty merge message" Kaartic Sivaraam
2017-07-06  4:46     ` Kevin Daudt
2017-07-06 12:20       ` Kaartic Sivaraam
2017-07-11 14:12       ` [PATCH] commit & merge: modularize the empty message validator Kaartic Sivaraam
2017-07-11 14:41         ` [PATCH/RFC] " Kaartic Sivaraam
2017-07-11 20:22         ` [PATCH] " Junio C Hamano
2017-07-13 13:00           ` Kaartic Sivaraam
2017-07-13 17:58             ` Junio C Hamano
2017-07-14 13:31               ` Kaartic Sivaraam
2017-07-17  9:08               ` Christian Brabandt
2017-07-17 17:16                 ` Junio C Hamano
2017-07-13 18:15           ` Kaartic Sivaraam
2017-07-13 19:23             ` Junio C Hamano
2017-07-14 17:49               ` Kaartic Sivaraam [this message]
2017-07-15  8:33                 ` Kaartic Sivaraam
2017-08-21 13:34                   ` [PATCH v2] branch: change the error messages to be more meaningful Kaartic Sivaraam
2017-08-21 13:52                     ` Kaartic Sivaraam
2017-08-21 14:05                   ` [PATCH v2/RFC] commit: change the meaning of an empty commit message Kaartic Sivaraam
2017-08-24 20:19                     ` Junio C Hamano
2017-08-31 13:36                       ` Kaartic Sivaraam
2017-10-02 17:20                         ` Kaartic Sivaraam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1500054552.18990.8.camel@gmail.com \
    --to=kaarticsivaraam91196@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ikke.info \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.