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

Log messages [was: [PATCH] Remove unreachable code in prompt.c]

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 05 Nov 2009 12:43:19 +0000

Edmund Wong wrote:
> Julian Foad wrote:
> > On Tue, 2009-11-03 at 22:28 +0800, Edmund Wong wrote:
> >> [[[
> >> Changed the ifdef WIN32 #endif block to an #ifndef WIN32 ... #else .. #endif
> >> block to avoid the 'unreachable code' warnings in Windows.
> >>
> >> * subversion\libsvn_subr\prompt.c
> >> wait_for_input: Changed #ifdef block to #ifndef block.
> >> ]]]
> >
> > Thanks for the patch. I committed it in r40371, tweaking it to also move
> > the comment that goes with the "return" statement.
> >
> > I wrote the log message as:
> > [[[
> > Put some code that is unused on Windows into a '#ifndef WIN32' block, to
> > avoid 'unreachable code' warnings.
> >
> > * subversion/libsvn_subr/prompt.c
> > (wait_for_input): As above.
> > ]]]
> >
> Thanks Julian for the help in the log. I still haven't gotten the hang
> of writing proper log messages. It's like there's so many different
> ways of expressing the changes, it's a bit hard.

It's an art, and there is no "correct" way. However, the guideline I
apply to myself is first to answer these questions:

  * Why make a change at all? (Bug fix? Enhancement?)
  * What result is intended? (Behaviour change? Benefit?)

This should give the reader (reviewer) enough high-level information to
judge whether the patch does what it is supposed to do. Then the next
section goes on to explain how the patch does what it does, if it is
complex enough to need further explanation.

In this patch, the important thing about the intent is that we are
hiding the code from the compiler, not exactly which preprocessor
directives we are using.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414694
Received on 2009-11-05 13:43:45 CET

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