Re: changelog format

[Stefano Lattarini]

On 05/25/2012 04:48 PM, Ludovic Courtès wrote:
> Hi Stefano,
> 
> Stefano Lattarini <address@hidden> skribis:
> 
>> On 05/02/2012 11:52 PM, Ludovic Courtès wrote:
>>
>>> 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 why would condensing the consensus/rationales reached on the list
into an extra paragraph in the ChangeLog entry be a bad idea exactly?

> and hopefully (partly) made clear in comments added to the code.
>
I never contested this should be done and advised; again, it is
complementary, not contrasting, with what I'm suggesting to do.

>>> 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,
>
But it wasn't: the motivation was clear and legitimate ("let's get rid of old
cruft"), the only thing still needed was someone verifying that what we were
removing was actually old cruft.  I did that, and explained and condensed my
findings for the sake of future developers, so that they wouldn't be forced
to go through the same VCS archeology/scavenging I did.

> and that’s enough;
>
I absolutely disagree.  Again, I want to refer you to what Samuel wrote here:
in <http://gcc.gnu.org/ml/gcc/2007-12/msg00016.html>, with which I heartily
agree.

> in addition, it should be obvious in this case, since the
> thing had been deprecated for ages.
>
But it took me ten/fifteen minutes to make sure of that (remember, I wasn't
around here when the original deprecation took place, so I wasn't sure since
since how long that usage had been deprecated, and how truly obsolete it
was).  Why force a future developer to waste the same time in case he wants
to double check the removal is warranted?

> At most, assuming the change has been discussed on the list,
>
No, it has just derived as a tangent from there.

> 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”.)
>
Than I don't understand what you mean, sorry.  Care to elaborate?

> 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’.

Regards,
  Stefano