On Fri, Feb 05, 2010 at 08:41:04PM +0100, Stefan Sperling wrote:
> 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.
A good algorithm. Perhaps it should be added to 'HACKING'. Although
there is *a lot* about writing log messages there. I admit to not having
studied it as detailed as I should have. Read it last summer and thought
I had grasped all of it but at a second look, that was not the case.
>
> 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?
Looks good. This sounds silly but I often feel a bit of rush when I'm
finished with a patch. It's like if I was thinking to myself: 'If I don't
get it in immediately, the bug may disappear by itself'. I'm trying
to slow down and give more thought to things. The most valueable lesson
so far from contributing to subversion is the importance of beeing
concious of all the choises I make when programming. In the end, writing
a log message should be pretty easy if I really understand what I've
been doing. :-)
Daniel
Received on 2010-02-05 21:02:18 CET