More verbose svn commit messages would be useful

[Roland Dreier <ro...@digitalvampire.org>]

Hi,

I think it would be nice if the project adopted the practice of using
svn commit messages that really describe the change being committed.
For example, r760437 just has:

    apply patch from johano fixing #31

and when I read this, I first have to know that #31 refers to JIRA,
then navigate to the JIRA page (which I might not be able to do if I'm
reading history while offline using a local mirror of the repo), etc.
I would have written something like:

    Clean up misspellings and javadoc warnings

    Patch from Johan Oskarsson (https://issues.apache.org/jira/browse/CASSANDRA-31)

As it stands, many of the commit messages are so terse that it's
really hard to have any idea of what code the commit touches, what the
change was, or why it was made.

I don't think we need any heavyweight rules -- just try to write
commit messages that make sense to someone who isn't already familiar
with the change being committed.

[I come from the Linux kernel world, where commit messages are
expected to be along the lines of (taking the latest commit in my
local tree as a random example):

    Build with -fno-dwarf2-cfi-asm
    
    With a sufficiently new compiler and binutils, code which wasn't
    previously generating .eh_frame sections has begun to.  Certain
    architectures (powerpc, in this case) may generate unexpected
    relocation formats in response to this, preventing modules from
    loading.
    
    While the new relocation types should probably be handled, revert
    to the previous behaviour with regards to generation of .eh_frame
    sections.
    
    (This was reported against Fedora, which appears to be the only
    distro doing any building against gcc-4.4 at present: RH
    bz#486545.)

and it really is helpful to be able to read the changelog and get an
overview of each patch]

 - R.

Re: More verbose svn commit messages would be useful

[Shalin Shekhar Mangar <sh...@gmail.com>]

On Tue, Mar 31, 2009 at 11:49 PM, Torsten Curdt <tc...@apache.org> wrote:

> > But my philosophy in general is that commit messages should be a one
> > sentance summary.  Maybe two.  Gory details should be in jira and the
> > code, if necessary.  The linux kernel may prefer to be more verbose
> > but that is not the only way to do things; I think you will find that
> > smaller projects lean towards being more terse.  I checked a few
> > projects I have checked out locally (sqlalchemy, thrift, hadoop core,
> > and hbase) and all seem to follow "one sentence or two."
>
> One sentence or two is fine ... as long as it captures nature of the
> commit.
>
> More important is to add the name of contributor of the patch (also
> for legal reasons) and it's more than just convenient to provide a
> link to the jira. That is common practice.
>
>
My 2 cents.

I guess if you follow the way commit messages are marked in the Lucene
world, it should be fine. For example:

CASSANDRA-31 -- Clean up misspellings and javadoc warnings

The CHANGES.txt is where all the authors of the contributions are marked.
You can add them to the commit message. But there are usually many people
contributing:
1. The person who reported the issue and/or opened the issue (perhaps only
on the mailing list)
2. The people who contributed patches
3. The people who reviewed the patch
4. The person who committed the code

For an example look at Solr's change log
http://svn.apache.org/repos/asf/lucene/solr/tags/release-1.3.0/CHANGES.txt

-- 
Regards,
Shalin Shekhar Mangar.

Re: More verbose svn commit messages would be useful

[Torsten Curdt <tc...@apache.org>]

> But my philosophy in general is that commit messages should be a one
> sentance summary.  Maybe two.  Gory details should be in jira and the
> code, if necessary.  The linux kernel may prefer to be more verbose
> but that is not the only way to do things; I think you will find that
> smaller projects lean towards being more terse.  I checked a few
> projects I have checked out locally (sqlalchemy, thrift, hadoop core,
> and hbase) and all seem to follow "one sentence or two."

One sentence or two is fine ... as long as it captures nature of the commit.

More important is to add the name of contributor of the patch (also
for legal reasons) and it's more than just convenient to provide a
link to the jira. That is common practice.

No biggy - but please keep that in mind next time :)

cheers
--
Torsten

Re: More verbose svn commit messages would be useful

[Roland Dreier <ro...@digitalvampire.org>]

 > But my philosophy in general is that commit messages should be a one
 > sentance summary.  Maybe two.  Gory details should be in jira and the
 > code, if necessary.  The linux kernel may prefer to be more verbose
 > but that is not the only way to do things; I think you will find that
 > smaller projects lean towards being more terse.  I checked a few
 > projects I have checked out locally (sqlalchemy, thrift, hadoop core,
 > and hbase) and all seem to follow "one sentence or two."
 > 
 > Do feel free to ask in jira (if applicable) or the mailing list if
 > you've looked at a commit and it doesn't make sense to you.  A good
 > svn client (sorry, the command line kinda sucks for this) will help.

Well, as I said, I'm not asking for a heavyweight process -- simply
enough information to understand and evaluate a commit.  I don't think
that relying on JIRA is a good idea, since commits from core
developers are just going into the subversion repository without
anything in JIRA or the mailing list.  And requiring JIRA descriptions
for every change is certainly a heavier weight process than just
putting a little more info into the changelog.

It's certainly the case that many smaller projects use very terse
commit messages, probably because the people with commit access know
the whole codebase and everything that is being worked on, and hence
don't see the need to describe things.  And larger projects by
necessity need to make it possible for developers to understand new
parts of the codebase.

But I really think it's in the project's best interest to describe
commits in "sufficient" detail.  Having a well-described development
history really makes the project more approachable and helps grow the
community.  And asking on JIRA or mailing lists really doesn't work --
assuming Cassandra succeeds, in a year or two you're going to be
looking at "svn blame" output, and see a commit that you yourself made
with a log message like "fix race condition", and no one will remember
what the race condition was.

In other words, don't underestimate how much you'll forget.  I've
definitely saved myself from spending a lot of time re-debugging a
problem that I already fixed once by being able to read my own
changelog entries for the kernel.  And I've even more often had to
refer back to the changelog to answer questions from others about why
something is done in a particular way.

Finally, when someone sends me a patch, I always need to be able to
evaluate the patch, and it simply doesn't work to expect me to reverse
engineer the patch itself -- there needs to be a changelog that
explains what the patch does, why we want to make that change, and any
other information (links to bug reports or discussions) that helps
evaluate the patch.  And since I expect that info from others, I
expect myself to provide it for any patches I produce.

Well, a lot of words about a small issue, but I guess as a kernel
subsystem maintainer, I spend a lot of time dealing with changelogs,
so I have strong opinions :)

 - R.

Re: More verbose svn commit messages would be useful

[Jonathan Ellis <jb...@gmail.com>]

I do think that a summary of the jira is appropriate and you may have
noticed that other than r760437 the commits do include that.
Apologies for the lapse.

But my philosophy in general is that commit messages should be a one
sentance summary.  Maybe two.  Gory details should be in jira and the
code, if necessary.  The linux kernel may prefer to be more verbose
but that is not the only way to do things; I think you will find that
smaller projects lean towards being more terse.  I checked a few
projects I have checked out locally (sqlalchemy, thrift, hadoop core,
and hbase) and all seem to follow "one sentence or two."

Do feel free to ask in jira (if applicable) or the mailing list if
you've looked at a commit and it doesn't make sense to you.  A good
svn client (sorry, the command line kinda sucks for this) will help.

-Jonathan

On Tue, Mar 31, 2009 at 11:37 AM, Roland Dreier
<ro...@digitalvampire.org> wrote:
> Hi,
>
> I think it would be nice if the project adopted the practice of using
> svn commit messages that really describe the change being committed.
> For example, r760437 just has:
>
>    apply patch from johano fixing #31
>
> and when I read this, I first have to know that #31 refers to JIRA,
> then navigate to the JIRA page (which I might not be able to do if I'm
> reading history while offline using a local mirror of the repo), etc.
> I would have written something like:
>
>    Clean up misspellings and javadoc warnings
>
>    Patch from Johan Oskarsson (https://issues.apache.org/jira/browse/CASSANDRA-31)
>
> As it stands, many of the commit messages are so terse that it's
> really hard to have any idea of what code the commit touches, what the
> change was, or why it was made.
>
> I don't think we need any heavyweight rules -- just try to write
> commit messages that make sense to someone who isn't already familiar
> with the change being committed.
>
> [I come from the Linux kernel world, where commit messages are
> expected to be along the lines of (taking the latest commit in my
> local tree as a random example):
>
>    Build with -fno-dwarf2-cfi-asm
>
>    With a sufficiently new compiler and binutils, code which wasn't
>    previously generating .eh_frame sections has begun to.  Certain
>    architectures (powerpc, in this case) may generate unexpected
>    relocation formats in response to this, preventing modules from
>    loading.
>
>    While the new relocation types should probably be handled, revert
>    to the previous behaviour with regards to generation of .eh_frame
>    sections.
>
>    (This was reported against Fedora, which appears to be the only
>    distro doing any building against gcc-4.4 at present: RH
>    bz#486545.)
>
> and it really is helpful to be able to read the changelog and get an
> overview of each patch]
>
>  - R.
>