Re: [PATCH] commit: abort before commit-msg if empty message

2 messages, 2 authors, 2018-12-11 · open the first message on its own page

Re: [PATCH] commit: abort before commit-msg if empty message

From: Junio C Hamano <hidden>
Date: 2018-12-08 05:44:21

Jonathan Tan [off-list ref] writes:
When a user runs "git commit" without specifying a message, an editor
appears with advice:

    Please enter the commit message for your changes. Lines starting
    with '#' will be ignored, and an empty message aborts the commit.

However, if the user supplies an empty message and has a commit-msg hook
which updates the message to be non-empty, the commit proceeds to occur,
despite what the advice states.
When "--no-edit" is given, and when commit-msg fills that blank, the
command should go ahead and record the commit, I think.

An automation where commit-msg is used to produce whatever
appropriate message for the automation is entirely a reasonable
thing to arrange.  Of course, you can move the logic to produce an
appropriate message for the automation from commit-msg to the script
that drives the "git commit" and use the output of that logic as the
value for the "-m" option to achieve the same, so in that sense,
there is an escape hatch even if you suddenly start to forbid such a
working set-up, but it nevertheless is unnecessary busywork for those
with such a set-up to adjust to this change.

I actually think in this partcular case, the commit-msg hook that
adds Change-ID to an empty message is BUGGY.  If the hook looked at
the message contents and refrains from making an otherwise empty
message to non-empty, there is no need for any change here.

In any case, you'll have plenty of time to make your case after the
rc freeze.  I am not so sympathetic to a patch that makes us bend
backwards to support such a buggy hook to e honest.

Re: [PATCH] commit: abort before commit-msg if empty message

From: Jonathan Tan <hidden>
Date: 2018-12-11 20:15:02

Jonathan Tan [off-list ref] writes:
quoted
When a user runs "git commit" without specifying a message, an editor
appears with advice:

    Please enter the commit message for your changes. Lines starting
    with '#' will be ignored, and an empty message aborts the commit.

However, if the user supplies an empty message and has a commit-msg hook
which updates the message to be non-empty, the commit proceeds to occur,
despite what the advice states.
When "--no-edit" is given, and when commit-msg fills that blank, the
command should go ahead and record the commit, I think.

An automation where commit-msg is used to produce whatever
appropriate message for the automation is entirely a reasonable
thing to arrange.  Of course, you can move the logic to produce an
appropriate message for the automation from commit-msg to the script
that drives the "git commit" and use the output of that logic as the
value for the "-m" option to achieve the same, so in that sense,
there is an escape hatch even if you suddenly start to forbid such a
working set-up, but it nevertheless is unnecessary busywork for those
with such a set-up to adjust to this change.
Thanks for bringing up this workflow. Note that this patch only changes
behavior when the editor is brought up and, thus, the advice is shown -
see the check for use_editor in prepare_to_commit(). So there should be
no change if --no-edit is given, but I acknowledge that there will be a
negative change if the user brings up the editor and just immediately
quits it (which can happen in a workflow where commit-msg always
produces an appropriate message, but the user can provide additional
information if desired).
I actually think in this partcular case, the commit-msg hook that
adds Change-ID to an empty message is BUGGY.  If the hook looked at
the message contents and refrains from making an otherwise empty
message to non-empty, there is no need for any change here.

In any case, you'll have plenty of time to make your case after the
rc freeze.  I am not so sympathetic to a patch that makes us bend
backwards to support such a buggy hook to e honest.
That's reasonable. In any case, we'll also look at fixing the Gerrit
hook - at the very least, so that it can work with versions of Git that
do not have this patch of mine (or something similar).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help