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

Re: [PATCH] v2 Test context within fuzzy hunk for 'svn patch'

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 5 Feb 2010 20:41:04 +0100

On Fri, Feb 05, 2010 at 08:03:46PM +0100, Daniel Näslund wrote:
> New log message which is ridiciously long for such a small change.

No, your new log message is much better! It's not ridiculous.

The basic algorithm for writing good log messages is:

1) Write the log message.
2) Think about *why* you are making the change.
3) Look for the answer to this question in the log message
   you've just written. If it is not there, go to step 1.
4) You're done.

The rest is partly intuition and partly practice.

You're almost right saying that really good log messages are short.
But it's not shortness that makes a log message good, but conciseness
(i.e. high information density).

> Feedback on it is welcome:
>
> [[[
> For determining fuzz we have an algorithm that should only check for
> trailing and leading context. Add a small regression test to existing
> testcase. At the same time fix small typo.

So this is better than what was there before (there was nothing before :)

If you make the second sentence refer to things mentioned in the first
one, it's even clearer what the regression test is for.
You can also easily say what kind of typo you've fixed:

 For determining fuzz we have an algorithm that should only check for
 trailing and leading context. Make an existing existing test case verify
 that this property of the algorithm holds. At the same time fix an
 indentation error.

And you can even merge the above text into this part of your
log message:

>
> * subversion/tests/cmdline/patch_tests.py
> (patch_with_fuzz): Add context in the middle of one of the fuzzy
> hunks.

because that function is the only scope of all your changes.
Which also shortens the log message a bit because now you can use the
pronoun 'this' to refer to the existing test case.
I'm also doing s/At the same time/Also/ because it provides the same
information but is shorter:

 * subversion/tests/cmdline/patch_tests.py
   (patch_with_fuzz): For determining fuzz we have an algorithm that
    should only check for trailing and leading context. Make this test
    verify that this property of the algorithm holds by adding context
    in the middle of one of the hunks. Also fix an indentation error.

What do you think?

Stefan
Received on 2010-02-05 20:41:41 CET

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.