Re: [PATCH] Renaming svn_eol_* functions to svn_eol__*.

[Edmund Wong <ed...@kdtc.net>]

Stefan Sperling wrote:
> On Sat, Aug 29, 2009 at 12:39:07PM +0800, Edmund Wong wrote:
>> Since the svn_eol_* functions are in the private
>> headers, Arfrever pointed out that they should be
>> renamed to s/svn_eol_/&_/g.
> 
> Some small remarks on the log message:

Even after all these submitted patches, I still can't seem
to get the log right.  Quite a disappointment I must say.  Sorry about
that.


> You might want to put a short note describing the overall change
> here, unless the change is really trivial. In this case, we could
> say:
> 
> "Follow-up to r38983: The new eol functions are supposed to be private,
> not public, so rename them accordingly."
>> * subversion/libsvn_subr/eol.c,
>>    subversion/libsvn_diff/diff_file.c,
> 
> The asterisk alone is a good enough visual hint for the grouping.
> So the indentation isn't needed, and these can all go on the same
> line, like this:
> 
> * subversion/libsvn_subr/eol.c,
>   subversion/libsvn_diff/diff_file.c,

I'll keep these in mind.

Edmund

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2389610

Re: [PATCH] Renaming svn_eol_* functions to svn_eol__*.

[Julian Foad <ju...@btopenworld.com>]

On Tue, 2009-09-01 at 09:58 +0800, Edmund Wong wrote:
> Stefan Sperling wrote:
> > On Sat, Aug 29, 2009 at 12:39:07PM +0800, Edmund Wong wrote:
> >> Since the svn_eol_* functions are in the private
> >> headers, Arfrever pointed out that they should be
> >> renamed to s/svn_eol_/&_/g.
> > 
> > Some small remarks on the log message:
> 
> Even after all these submitted patches, I still can't seem
> to get the log right.  Quite a disappointment I must say.  Sorry about
> that.

Pah. You're doing fine. Better than fine. We're not disappointed, but
rather extremely appreciative of your patches and log messages.

It's really really helpful for every log message to start with a brief
statement of the purpose of the change. That's important. It can make
the difference between other people being able to understand the change
and not being able to understand it.

It's also good to be tidy and consistent with spaces and colons and
commas and whatnot, but that's on a much, much lower level, and the
space taken up talking about in emails it is far greater than its
importance. It makes a small difference to readability but doesn't
usually affect the reader's understanding.

In cases like this patch of yours, experienced committers sometimes
judge that the purpose is so obvious that it's not worth mentioning, but
to avoid uncertainty it's better to mention it anyway.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2389719

Re: [PATCH] Renaming svn_eol_* functions to svn_eol__*.

[Stefan Sperling <st...@elego.de>]

On Tue, Sep 01, 2009 at 09:58:56AM +0800, Edmund Wong wrote:
> Stefan Sperling wrote:
> > On Sat, Aug 29, 2009 at 12:39:07PM +0800, Edmund Wong wrote:
> >> Since the svn_eol_* functions are in the private
> >> headers, Arfrever pointed out that they should be
> >> renamed to s/svn_eol_/&_/g.
> > 
> > Some small remarks on the log message:
> 
> Even after all these submitted patches, I still can't seem
> to get the log right.

Don't worry.

Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2389723