[parted-devel] [PATCH 1/2] libparted: enforce dos partition limit

[Phillip Susi]

On 1/18/2012 8:12 AM, Jim Meyering wrote:
> Thanks for the fix and added test.  And NEWS.
> Almost perfect.
> I've appended this to the commit log:
> 
>     * NEWS (Bug fixes): Mention it.
>     * libparted/labels/dos.c (next_primary): Return -1 upon failure.
>     (next_logical): Stop no later than MAX_TOTAL_PART.
>     Throw exception and return -1 upon failure.
>     (msdos_partition_enumerate): Convert a negative partition number
>     return value from either of the above two to failure (return 0).
>     * tests/t9042-dos-partition-limit.sh: New file.
>     * tests/Makefile.am (TESTS): Add it.

This bullet point style of logging was popular and probably necessary
with CVS because it is such a horrid tool, but I dislike it with modern
VCS.  I think that the log should provide a general overview of what the
changeset does and why, not detail every little line by line change (
that's what the diff is for ).  For instance, these lines add nothing of
value and only serve to clutter:

>     * NEWS (Bug fixes): Mention it.
>     * tests/t9042-dos-partition-limit.sh: New file.
>     * tests/Makefile.am (TESTS): Add it.

It is clear from the diffstat that the test file was added and one can
readily assume ( and clearly see at a glance in the diff if inclined to
look at it ) that the test was also added to the makefile.  When you see
the NEWS file listed in the diffstat you can also readily assume an
entry was added describing this change.  Explicitly mentioning it in the
log doesn't help one to understand the commit.  With modern tools like
gitk, that line by line level of detail is only a click away, so leaving
them out of the change log helps greatly when reviewing the log of
multiple changes as there is less clutter.