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

Re: svn commit: r1649029 - in /subversion/trunk/subversion/libsvn_fs_x: ./ dag.c tree.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 12 Jan 2015 16:08:27 +0000

Ivan Zhakov wrote:
> On 12 January 2015 at 18:39, Julian Foad wrote:
>> Ivan Zhakov wrote:
>>>>>> URL: http://svn.apache.org/r1649029
>>>>>> Log:
>>>>>> Sync FSX with FSFS: Merge DAG-related sub-pool introduction
>>>>>> patches r1647905, r1648243, r1648253 and r1648272 from FSFS
>>>>>> and resolve the usual text conflicts due to naming differences.
>>>>>> Block revisions r1648230, r1648238, r1648241, r1648242 and r1648532.
>>
>>> Also, it seems that this commit has another problems:
>>> 1. It doesn't have proper log message [1].
>>> 2. It mixes different unrelated changes in one commit, while it will
>>> be much easier to review them separately.
>>
>> There is nothing wrong here. The log message says what's merged, and
>> that is four revisions all described as "DAG-related sub-pool
>> introduction", which means they are all parts of the same change - not
>> unrelated changes.
>
> Subversion community guide is very specific about proper log messages
> and I don't understand why this commit (and other previous FSX "merge"
> commits") does not follow it.

The Community Guide doesn't document what we do for merges. For merges, we nearly always just say what was merged. In my opinion, giving a detailed log message for what's changed by a merge should be encouraged but not required. I would suggest that for a fairly simple, mechanical change like this one, it is not usually very useful.

> Quoting Subversion community guide [1] again:
> [[[
> The log message should name every affected function, variable, macro,
> makefile target, grammar rule, etc, including the names of symbols
> that are being removed in this commit. This helps people searching
> through the logs later. Don't hide names in wildcards, because the
> globbed portion may be what someone searches for later.
> ]]]

Some exceptions are listed after the quoted section. Merges are another exception, which isn't (yet) documented.

> There is very good reason that every affected identifier should be
> mentioned in log message.

Sure, but not for every kind of change.

- Julian

> [1]
> https://subversion.apache.org/docs/community-guide/conventions.html#log-messages
Received on 2015-01-12 17:11:24 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.