Re: New Git hooks for checking file names in commit messages

[Eli Zaretskii]

> Date: Sat, 22 Apr 2023 12:52:58 -0700
> Cc: arsen@aarsen.me, acm@muc.de, emacs-devel@gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 4/22/2023 12:03 AM, Eli Zaretskii wrote:
> >> From: Björn Bidar <bjorn.bidar@thaodan.de>
> >> Should branches be checked by these hooks before they are merged into
> >> master/version branches?
> > 
> > No.  We don't enforce these rules on branches from which Emacs
> > tarballs will not be produced.  The commit log message which matters
> > for those is the one of the merge-commit when the branch is landed on
> > master.
> I'll make sure this works. I do think it's useful to check commits on 
> all branches, since we won't know until the merge how a committer plans 
> to do the merge.

The risk exists, but the freedom we give developers with log messages
on feature branches is more important.  We should not make developing
features more troublesome by enforcing well-formatted log messages for
WIP changes, especially since frequently the commits are just
checkpoints, they don't signify any changes that have a useful
description.  So we should not block pushing of branch commits due to
log messages.

> For example, you could push a feature branch upstream, and when the
> time comes to merge it, rebase it onto master. In that case the
> history is linear, so we'd want to check every commit that was
> newly-added to master.

We prefer that people do not rebase onto master.  We hope they don't
do that, in practice.  They can rebase the branch before merging it,
of course, but that is not the situation you describe, and should not
cause problems with these hooks, because only the log message of the
final merge-commit matters.

> I'm not exactly sure how the Git log -> ChangeLog -> etc/AUTHORS process 
> works for merge commits

Merge commits usually don't have any meaningful log messages and don't
mention files.  You must explicitly add a log message mentioning files
and functions for a merge-commit to have a log message, and we request
that people who land feature branch create a log message describing
all the new/changed things in the merge for that single commit before
you push.  Most other merge-commits aren't important in the context of
this discussion.

> For example, we could just detect if we're pushing a merge commit
> and then stop validating any commits-to-be-pushed prior to that.

If this can be done easily and without mistakes, that would be okay, I
think.  But please consider all the frequent use cases where
merge-commits appear.  These include:

  . merge-commit by "git pull" when there are local commits (several
    people making changes to the same branch at the same time)
  . merge from the release branch to master
  . cherry-pick from master to the release branch
  . merge of a feature branch or a scratch branch

> Of course, we could also say that it's still useful to validate all 
> those commits anyway, since it keeps our commit history better-formatted.

But if so, what do you want the user who does the merge do?  The log
messages of past commits cannot be amended, so this would simply
preclude people from pushing merges and/or force them to use the
"--no-verify" switch.  For example, a frequent situation for merges is
a merge from the release branch to master -- if one or more of the
commits being merged has bad log messages, we don't want to block the
merge, because nothing can be done about those bad messages at the
time of the merge.

> If there's ever a case where these new hooks do the wrong thing, we can 
> just run "git push --no-verify" to ignore the hooks. We're the humans, 
> so we have the final word over the computer, after all. :)

Increasing the number of situations where we'd need --no-verify is not
a Good Thing.  In this case, it is better to leave the badly-formatted
log messages alone, and let them be dealt with at make-tarball time.