Re: ChangeLog and commit messages

[Konstantin Kharlamov]

(I apologise in advance, I likely to be slow to answer)

On Mon, 2023-06-19 at 18:10 +0800, Po Lu wrote:
> Konstantin Kharlamov <hi-angel@yandex.ru> writes:
> 
> > (actually, it turns out I did work with SVN in my first job. But it wasn't
> > the
> > only VCS we used, so I remember close to nothing about that 😅)
> 
> I'm thinking about RCS and its relatives.
> 
> > Thanks, I see this commit, I'm looking.
> 
> There are many different commits with this title.  The last change was
> small, but others cannot be split up: look near the start of the branch
> point for an example.

Okay, so, it might be more efficient if you point me at a specific commit you
have in mind. Does this one work, for example?
https://git.savannah.gnu.org/cgit/emacs.git/commit/?h=feature/android&id=5b4dea0fc781fe40548e7b58fe5bd201a05f3913

> BTW, this brings up another problem with Git: it isn't possible to
> determine the branch point (or even the branch name) of a revision from
> the name of that revision alone.

Oh, yeah. I imagine this could come in handy, for example to remove merged local
branches. I have so many at work, but it's often very hard to automatically
determine which ones had been merged, because commits ID often change upon merge
through rebase.

OTOH, if a project have many thousands of commits, like Linux kernel, this may
become a problem as your name may clash with another one for legitimate
reasons. And I also had on a few occasions multiple branches where commit with
the same title had different content (again, for legitimate reasons: I was
experimenting with different approaches, and only one of these commits made it
upstream in the end. Final commit likely had title changed, but I still have
other "testing" commits locally).

So… I would say whether it's better is debatable.

> > So… From what I can see you have two separate changes in this commit. One is
> > "Only access width and height under lock", and another one… it is some
> > algorithmic change in the `onLayout()`, which I didn't study too closely,
> > but
> > looking at the commit description I'm assuming it is "Send Expose upon
> > layout
> > changes if the view has grown".
> > 
> > The problem you have here with making up a title is because you've got two
> > different functional changes inside one commit. Note that you had no trouble
> > writing what has changed inside the commit message. So, had you separated
> > the
> > commit to two different ones, you would've gotten a commit title for free
> > just
> > by looking at your commit message 😊
> > 
> > When someone mixes up different functional changes, all they can give as a
> > title
> > is just "Refactor foo.c" or "Rework bar" or "Fix buzz". When later looking
> > at
> > the shortlog these are pretty unhelpful titles, because it often says very
> > little about what really was done in the code.
> 
> Which is the problem with the ``shortlog'' concept: detailed
> descriptions of these changes should be placed in ChangeLog, and the VCS
> should concentrate on tracking revisions to each file.

Sorry, I may be missing something. You said "this is the problem" which implies
the problem should be obvious from the context, but I struggle to see it. Some
additional details may help.

We can discuss a workflow with placing entries to a Changelog file as compared
to usual git workflow, if you want. I'm just not sure if you're interested in
that.

> > It is worth noting that although Emacs does allow mixing up many changes in
> > one,
> > a generally accepted good practice is to avoid that. Separating changes
> > allows
> > for the author to easier figure out what went wrong, it makes review easier,
> > and
> > when one have to revert an offending commit, it makes sure that unrelated
> > good
> > changes won't be reverted together with the ones that broke something. In
> > many
> > large projects such mixing up is not allowed and in code review the author
> > will
> > be asked to separate the changes.
> 
> I don't think so.  At my organization, which uses SVN, each new revision
> is required to change only one file, and updates to ChangeLog are
> performed separately after all edits are checked in and the
> corresponding files are unlocked.  This is also the same policy that was
> used by Emacs development in the CVS days, I think.

I am afraid such workflow would break both bisection and the ability to see
what's changed by other developers. This is because in many cases a change
affects more than one file. For example: when you change a function helper in C,
you may need to edit a header and all C files where it's declared. If you create
separate revision for each of these changes, this will α) break compilation in
each of these revisions till the last one, and β) will make reading the log
(either for code review or for other purposes) harder, because the context of
the change is no longer in the same commit.

FWIW, at my work after stumbling upon a few situations where some commits
couldn't be compiled within same series, we ended up adding a CI check that
tests each individual commit for compilation. So people can't commit a revision
that does not pass compilation.

> I don't know how to do this with Git, since it's not possible to update
> ChangeLog separately from file check-ins.

I might be missing something here, because I'm not sure what is not possible
here. I mean, you can create a separate commit for each individual file. And you
can edit Changelog there as well, or you can avoid editing it.

> > P.S: btw, in the article² by a kernel maintainer that I referred elsewhere
> > there's also a good quote on your case:
> > 
> > > If it's not possible to describe the high level change in a few words, it
> > > is
> > most likely too complex for a single commit
> > 
> > 1: https://gitlab.freedesktop.org/mesa/mesa/-/commits/main
> > 2: http://who-t.blogspot.com/2009/12/on-commit-messages.html
> 
> I've read the article, and I don't agree.

If you have any specific points, we can discuss them, if you want of course.