?

Log in

No account? Create an account
 
 
19 May 2009 @ 10:11 pm
Commit message formatting  
(or: GNU coding standards considered harmful)

I've recently been watching GCC and GDB development, and every time I have reason to go looking through their commits, the total lack of useful information in the commit messages really strikes me.

Take this commit message for example, reproduced below for your convenience:

Log message:
	* fork-child.c: Don't include frame.h.  Include terminal.h.
	(fork_inferior): Call new_tty_postfork after forking adn adding
	the child to the inferior list.
	* inferior.h (new_tty_prefork, gdb_has_a_terminal): Don't declare
	here.
	* inflow.c (struct terminal_info): Remove const qualifier from
	`run_terminal' field.
	(inferior_thisrun_terminal): Tweak comment.
	(inflow_inferior_exit): Release the `run_terminal' field.
	(copy_terminal_info): New function.
	(new_tty_postfork): New function.
	* terminal.h (new_tty_prefork, new_tty, new_tty_postfork,
	(copy_terminal_info, gdb_has_a_terminal, gdb_setpgid): Declare.
	* inf-ptrace.c: Include terminal.h.
	(inf_ptrace_follow_fork): Copy the parent's terminal info to the
	child.
	* linux-nat.c: Include terminal.h.
	(linux_child_follow_fork): Copy the parent's terminal info to the
	child.
	* inf-ttrace.c: Include terminal.h.
	(inf_ttrace_follow_fork): Copy the parent's terminal info to the
	child.



Okay, so quick quiz: what does this change do? Does it fix a bug? Add a feature? Can you tell from this message? If you get lost in the useless meaningless text, well, me too.

What if instead of showing you that, I showed you this email and this email about an earlier part of the same change. Isn't that much better? Wouldn't it be nice if I'd shown you that to begin with?

Before you think I'm just picking on the maintainers of gdb, let me now point you to the GNU coding standards changelog guidelines, which they're faithfully following.


There's no need to describe the full purpose of the changes or how they work together. If you think that a change calls for explanation, you're probably right. Please do explain it—but please put the explanation in comments in the code, where people will see it whenever they see the code. For example, “New function” is enough for the change log when you add a function, because there should be a comment before the function definition to explain what it does.


Let's think about this...gdb (and all the rest of the GNU projects) are using a version control system. Even one as poor as CVS can tell me that a function is new, when it was added, and who did it. So exactly what value is putting "(new_tty_postfork): New function." in the changelog? Absolutely none. Did I really need the author to tell me that a new .h file was being included now? No, of course not, even if that might be interesting for some reason, it's utterly obvious from the diff.

The GNU coding standard is being actively harmful here, by recommending that people put useless junk in the change message, instead of an explanation of the change. It's really rather unfortunate.

What do I want to see in a commit message? Well, it seems to me that basically everyone except the GNU project has this well figured out already, but:

At a high level: what does the change do (implementation), and what is it for (reason). Include both a one-line summary of the change, and then some prose describing it in more detail. Try to keep it high level: in most cases, the details about each individual thing you touched in your change isn't necessary. I should be able to get that from the diff directly.
 
 
 
(Anonymous) on May 21st, 2009 05:37 am (UTC)
The purpose of ChangeLog
It seems people have missed the point of what a ChaneLog (CL) entry is
supposed to document.

As the GNU Coding Standards state, it is not supposed to document the
`why', but what happened to the source tree. This is so you can get a
overall view of what changes the patch does or did; if you get a large
patch you will want to have a brief summary of what it does (not the
why, but code changes). Just doing a diff, and getting a monstrous
patch back will be quite useless, but a few CL entries will make life
easier in tracking down a newly introduced bug.

Having the `why' documented, or more precisly, the intent of the code,
is very important as well, but this belongs in the actual code, or in
the manual.

Writing a CL entry is not that hard either, and gives you a overview
over your own changes. I have often seen better ways to fix a bug
after having written a CL entry.

Whether one agrees with this or not is really beside the point, but a
CL entry does not document the `why', it never had this intent. It is
a general overview of a change, something that diff will never be able
to give you.

Alfred M. Szmidt <ams@gnu.org> -- A GNU Hacker who cannot live without
ChangeLog.
fuhm on May 21st, 2009 04:43 pm (UTC)
Re: The purpose of ChangeLog
Having the `why' documented, or more precisly, the intent of the code, is very important as well, but this belongs in the actual code, or in the manual.


You're missing the distinction between the why of how the code currently works, and why the change was made. Absolutely the intent of the code should be documented in the code. However, the intent of the change should be documented in the commit message.