Re: changelog format

[Ludovic Courtès]

Hi Stefano,

Stefano Lattarini <address@hidden> skribis:

> On 05/02/2012 11:52 PM, Ludovic Courtès wrote:
>> Hi!
>>
> Hi Ludo.
>
>> (Resending a message that Gmane had apparently swallowed.)
>> 
>> Stefano Lattarini <address@hidden> skribis:
>> 
>>> Then the ChangeLog is mostly useless, and writing it mostly a waste of time.
>>> I'm not the fist one to reach this conclusion:
>>>   <http://gcc.gnu.org/ml/gcc/2007-12/msg00016.html>
>>>   <http://thread.gmane.org/gmane.comp.gcc.devel/94464/>
>> 
>> That’s the great misunderstanding about change logs.

[...]

>> What, to me, summarizes the rationale best is this paragraph (info
>> "(standards) Change Log Concepts"):
>> 
>>      There's no need to describe the full purpose of the changes or how
>>   they work together.
>>
> Which, followed to the letter, would make the ChangeLog entries basically
> worthless for all but the most trivial commits.

That’s what I consider to be a misunderstanding.  Change logs describe
changes; you may consider it worthless, but I find it valuable, in
particular when reviewing code.

Now, sure, that’s not enough.  You also want to know the rationale for a
change, and that’s typically discussed on the mailing list, and
hopefully (partly) made clear in comments added to the code.

>>   However, sometimes it is useful to write one line
>>   to describe the overall purpose of a change or a batch of changes.  If
>>   you think that a change calls for explanation, you're probably right.
>>   Please do explain it--but please put the full explanation in comments
>>   in the code, where people will see it whenever they see the code.  For
>>   example, "New function" is enough for the change log when you add a
>>   function, because there should be a comment before the function
>>   definition to explain what it does.
>> 
>> Indeed, it’s more convenient when the explanation is next to the code,
>>
> It depends.  The how and why the code works the way it works -- yes, that
> must definitely must go in code comments (albeit in most cases, placing
> also an "abridged" version placed in the commit message won't hurt).  But
> the rationale of why a change is needed, and how it relates to previous
> changes or attempts -- that must go in the commit messages (albeit in
> some cases, reporting part of that history in code comments can be of
> great help).

Often, those who read code are interested in an explanation for the
current status, as opposed to an explanation of the chain of changes
that led to the current status.

In some cases, though, the changes themselves need particular
explanation that would not fit in a comment in the code.  In that case,
I agree that it’s good to do that.

>> rather than buried in the VCS history.
>>
> The two approaches are not mutually exclusive, but complementary: some
> comments goes in the VCS history, some in the code/documentation, and
> some in both places.

Yes, I use ‘C-x v g’ regularly, but in some cases, that’s because the
code lacks comments.

>> And change logs serve a different purpose: they give a higher-level view
>> of the diff, sort-of like a semantic patch.
>>
> This is a good aspect, but it complements rather than substitute the
> higher-level explanation and motivation of a change.  Let me take a
> recent Automake-NG commit as an example:

[...]

> SUMMARY LINE
>
>     [ng] config.h.{bot,top}: don't support anymore (distribution and deps)
>
> This says in a line what the commit does.

Yes, I agree that this is useful (for Guile, we use Git and use commit
logs to store ChangeLog-style info; obviously, there’s always such a
summary line.)

> MOTIVATION AND REFERENCES
>
>     The use of those files have been obsoleted since Autoconf commit 5047ea80
>     of 1994-08-09, "support alternate input file names"; yes, the "1994" in
>     there is not a typo: those files were already deprecated in Autoconf 2.0.
>     It's well past time to remove support for them!
>
>     For more information, see chapter "Obsolete Constructs", section
>     "acconfig.h" of the Autoconf manual.  See also the discussion on automake
>     bug#7919, in particular the message <http://debbugs.gnu.org/7819#20>.

IMO, the motivation should have been discussed on the mailing list, and
that’s enough; in addition, it should be obvious in this case, since the
thing had been deprecated for ages.

At most, assuming the change has been discussed on the list, I’d have
written at most something like:

  This feature had been deprecated since 1994.  See
  <http://debbugs.gnu.org/7819#20> for details.

Or, if it fixes that bug:

  Fixes <http://debbugs.gnu.org/7819>.

As a reviewer, I really like it extra comments go straight to the point,
without duplicating a discussion that has already taken place.

> DETAILED PER-FILE CHANGES
>
>     * NG-NEWS: Update.
>
> This is just noise, but is mandated by the GCS, and since consistency
> is good, I'd rather have it than not.
>
>     * automake.in (handle_configure): Don't automatically distribute the
>     'config.h.top' and 'config.h.bot' files if they exist, and don't add
>     them to the '%FILES%' transform when processing the 'remake-hdr.am'
>     Makefile fragment.  In fact, drop the '%FILES%' transform altogether,
>     since now it would always expand to empty.
>     (@common_sometimes): Don't list 'config.h.top' and 'config.h.bot'
>     anymore.
>     * lib/am/remake-hdr.am (%CONFIG_HIN%): Don't depend on '%FILES%'
>     anymore.  That transform has been removed now (and wouldn't be needed
>     anyway).
>     * t/autodist-config-headers.sh: Remove as obsolete.
>
> This has the role of "semantic patch" you was referring to.  Don't just
> report the single diffs (which already present in the "git diff" output
> anyway), but explain why they are needed / makes sense.  True, there is
> some duplication with what "git diff" would tell us anyway, that that's
> acceptable (and as I've already stated in a previous message, having
> some redundancy when explaining things to a human usually makes more
> good than harm).

I would not write any rationale such as “wouldn’t be needed anyway”.  I
would really make it a diff of the abstract syntax trees (that’s what I
meant by “semantic patch”.)

So, to sum up, I would write any rationale primarily in comments in the
code, or, if that’s unsuitable, as a /few/ lines above the detailed
per-file changes.

Thanks,
Ludo’.