[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: CVS update: subversion/subversion/libsvn_fs dag.c dag.h

From: Karl Fogel <kfogel_at_galois.collab.net>
Date: 2001-02-22 15:36:33 CET

Jim Blandy <jimb@zwingli.cygnus.com> writes:
> This is another example of why I think log messages should include
> filenames. If I'm just looking at the log, and I haven't gone and
> pulled up the patch yet, I can't tell whether svn_fs__dag_close was
> removed from dag.c, dag.h, or both.

I wonder about this, actually. IMHO it's overkill for log messages to
always link each changed symbol with the exact set of files it changed
in -- it's asking something of log messages that they're not meant to
do.

I don't think we need a hard and fast rule that log messages should
link symbols to files. Doing so can interfere with the message's
readability (due to imperfect overlaps, one sometimes cannot avoid
either mentioning a file twice or mentioning a symbol twice), and the
information can be gotten another way without too much effort if one
really needs it. I find that I usually don't need it, either; or
rather, that I end up discovering it painlessly in the normal process
of coding.

There's always a continuum between the log message and the diff
itself. The question is how far along that continuum one should go --
if you go too far, the log message loses brevity; if you don't go far
enough, the log message isn't useful.

That question is best answered by looking at the purpose of a log
message. I think the purpose we've been using is something like this:

   A log message should summarize the change, so someone reading the
   diff has an overview to guide them. The log message should usually
   also mention every affected symbol, so if someone is trying to find
   out when a certain specific change was made -- say, a function or
   variable was introduced or deleted -- the log helps them narrow it
   down very quickly. (This latter requirement is relaxed for large,
   sweeping changes, however.)

So when you say

> If I'm just looking at the log, and I haven't gone and
> pulled up the patch yet, I can't tell whether svn_fs__dag_close was
> removed from dag.c, dag.h, or both.

...I think "Yes, but the log message is doing enough to help you find
out what you need to know." That is, it mentions the symbol, and says
it was removed. It also mentions all the affected files -- dag.c and
dag.h, it just doesn't *guarantee* that the `svn_fs__dag_close' change
happened in both files. However, one can guess pretty reliably that
Ben removed it from both files, because that would be the sensible
thing to do, and the project is, after all, under version control. :-)

If you do an update and notice the function is gone from somewhere,
and you wonder why it's gone, then this log message tells you
everything you need to know. If you're browsing recent commits, this
log message also tells you what you need to know. If you're looking
at the diff, this log message tells you everything you need to know.

-K

> sussman@tigris.org writes:
>
> >
> > User: sussman
> > Date: 01/02/21 09:03:42
> >
> > Modified: subversion/libsvn_fs dag.c dag.h
> > Log:
> > (svn_fs__dag_close): removed for now. we're not currently worried
> > about freeing memory at the resolution of individual dag nodes.
> >
> > (node_is_kind_p, svn_fs__dag_is_file, svn_fs__dag_is_dir,
> > svn_fs__dag_is_copy): implement.
Received on Sat Oct 21 14:36:23 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.