Re: [PATCH] release.py: write-changelog function (POC)

On Thu, Dec 14, 2017 at 6:58 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> Johan Corveleyn wrote on Mon, Dec 04, 2017 at 12:16:37 +0100:
>> As promised on IRC last Friday, here is a POC implementation of the
>> "generate changelog from commit messages" functionality,
>
> We had some more discussion about this on IRC last Monday¹; I'll try to
> summarize.

Thanks, Daniel. Sorry I didn't get around to dev_at_-summarizing ...
Some comments below.

...
> Personally, my answers are^W^W proposal is:
>
> - Use a whitelist approach. To ensure everything noteworthy gets mentioned, we
> look for missing CHANGES taglines in log messages as part of our ordinary
> post-commit review of commits@ traffic. The rationale here is not to encumber
> trivial commits with the need to explicitly opt-out.

Okay, I like the whitelist approach most too. The fact that for 1.10
we only have 244 CHANGES out of 1633 log-message-summaries (less than
1 out of 6) indicates that most commits won't have anything to say
"externally". So I follow your point on "not encumbering trivial
commits". OTOH, I can also live with the blacklist approach as a way
to make sure the RM certainly doesn't miss things, if others would
insist.

> - We invent some convention (possibly, but not necessarily, a parseable one) to
> explicitly indicate "This commit shouldn't be added to CHANGES", in order to
> avoid people wondering whether the omission of a CHANGES tagline was
> deliberate or an oversight. (I believe this aligns with Stefan Hett's
> $WORK's experience with this convention.)
>
> Thus, every significant commit should have either a CHANGES tagline or
> CHANGES opt-out tag; and if a significant commit has neither, then we ask
> about that in a post-commit review and, if needed, propedit the log message
> to add a CHANGES tagline.

I'd like to add (a heuristic to determine "significance" if you will):
log messages without a summary, usually parseable by seeing a "*" as
the first character, are implicitly "not interesting for CHANGES". No
need to exclude them explicitly. We could also add a couple of other
conventions (I did this in my POC patch, with the --pocfirstlines
option): having STATUS or CHANGES in the summary line usually
indicates: not interesting for adding to CHANGES.

> - The CHANGES tagline can be either a separate line in the log message, or the
> first line of the log message. The rationale here is that the target
> audiences are different, so sometimes the descriptions should be different,
> but when the descriptions happen to be identical, avoid duplicated effort by
> the log message author. For example:
>
> [[[
> svnsync: Make everything better. U[client]
>
> * subversion/svnsync/svnsync.c
> ⋮
> ]]]
>
> [[[
> ra_svn: Fix a 30 years old bug in the protocol version number comparison
> logic.
>
> U[client and server]: ra_svn: Restore interoperability with 1.42.x, broken by 1.42.3.
>
> * subversion/libsvn_ra_svn/greeting.c
> ⋮
> ]]]

+1, I like having both options (CHANGES-ify the summary, or write
CHANGES entry on a separate line).

> - For the syntax, U[client] for "User-visible changes"/"Client-side bugfixes"
> and similar syntaxes for the other CHANGES groups. The parser will look for
> U[…] and D[…] where the ellipses stand for zero or more words. We'll
> probably come up with a standardish set of abbreviations here, reflecting the
> categorisation of items in CHANGES. The syntax for "explicitly not for
> CHANGES" can then be the words "Not for CHANGES." as a standard formula (like
> "No functional change." is a standard formula) or some parseable thing (e.g.,
> "U[no]").
>
> Whenever the CHANGES tagline is not the first line of the log message, we
> might find it useful to give it some constant prefix, e.g.,
> .
> [[[
> ra_svn: Fix a 30 years old bug in the protocol version number comparison
> logic.
>
> c: U[client and server]: ra_svn: Restore interoperability with 1.42.x, broken by 1.42.3.
> ]]]
> .
> for easier / unambiguous parsing.

I must say I still prefer [U:client], [U], [no] or [ignore] over
U[client] etc. The first reason is that I find it more easily
parseable / recognizable as a human reader, but I admit this is very
subjective. Another reason is that I think it eliminates the need to
add that constant prefix like "c: " to make it stand out on a separate
line. I think having a line start with '[' immediately makes it
special enough (both for human readers as for parsers).

      [[[
      ra_svn: Fix a 30 years old bug in the protocol version number comparison
      logic.

      [U:clientserver]: ra_svn: Restore interoperability with 1.42.x,
broken by 1.42.3.
      ]]]

Finally: something else I mentioned on IRC a couple of days ago: if we
assume a maximum line length of 80 characters in CHANGES, we have only
57 or 63 characters available for a "CHANGES entry", because of the '
  * ' in front, and the ' (r1234567)' or ' (r1234567 et al)' at the
end. Don't know if we need to do something about that, or just "let
CHANGES go over 80 characters if your summary is 70-character-ish ...
whatever".

-- 
Johan