emacs-devel
[Top][All Lists]
Advanced

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

Re: Mistakes in commit log messages


From: Jim Porter
Subject: Re: Mistakes in commit log messages
Date: Tue, 11 Apr 2023 12:27:40 -0700

On 4/11/2023 11:45 AM, Eli Zaretskii wrote:
Date: Tue, 11 Apr 2023 11:31:55 -0700
Cc: emacs-devel@gnu.org
From: Jim Porter <jporterbugs@gmail.com>

Finally, I think it would make sense to have this be a purely advisory
warning for now so that we could check it into the Emacs tree soon-ish.

How will this help, given that some people use Magit to commit and
some use VC, not the Git commands from the shell?  Are we sure the
warnings will be seen, let alone acted upon?

I don't know. If VC and Magit don't show messages from Git hooks, they probably should. Those messages are there for a reason, after all. However, warnings that only some users see would hopefully still help us get feedback on whether the validation works properly.

Some miscellaneous thoughts on this:

One issue with the current implementation (mentioned elsewhere in this thread) is that it doesn't work for deleting a file from the repo. Fixing this in the commit-msg hook is tricky. We can get the list of changed files via "git diff --name-only" (and then we could compare our commit message against that list), but there's a problem: I don't know of a good way to detect when the user is amending a commit[1]. For a normal commit, you'd get the changed files via something like "git diff --staged --name-only", but for an amended commit, you'd want to add "HEAD^" to that command.

Another option might be to do this as a pre-push hook. By then, the commits are finalized, so we can more easily compare the commit message and its diff. This is also nice because when the commit-msg hook errors out, it throws away the commit message; quite annoying if you just made a small typo! However, it does mean that committers would need to be comfortable with amending their commit messages if they hit this error.

(We could also check this in a post-commit hook so that committers are alerted *before* they try to push, but post-commit hooks can't abort the commit: it's already done! Maybe we could have a post-commit hook that warns the committer, and a pre-push hook that actually errors out.)

[1] The only way I've seen to do this is to look at the arguments from the parent (Git) process that called the hook: <https://stackoverflow.com/questions/19387073/how-to-detect-commit-amend-by-pre-commit-hook>. I'm not sure how reliably that works, especially on platforms like MS-Windows...



reply via email to

[Prev in Thread] Current Thread [Next in Thread]