svn commit: r278718 - in stable/9: etc/rc.d sbin share/man/man4 share/mk sys/modules/geom tools/build/mk tools/build/options

[John Baldwin]

On Monday, February 23, 2015 01:41:16 PM Justin T. Gibbs wrote:
> > On Feb 23, 2015, at 12:07 PM, John Baldwin <jhb at freebsd.org> wrote:
> > I'm coming at this from a different angle I guess.  When I was first using
> > FreeBSD, I didn't read HEAD commits, but I did follow commits for
> > 2.2-stable. What I cared about as a user of stable was what new features
> > were coming into 2.2.  I didn't really care about the details of the
> > various bugfixes to get said feature into mature shape in HEAD that were
> > bundled into a merge, I just cared about the new feature itself.  That
> > is, I'm assuming the reader _hasn't_ already read this commit before, but
> > that the extra noise makes it overly verbose.  I think these are only
> > readable _if_ you've already read the stream of commits to HEAD prior so
> > it is just a refresh of what's in your memory vs something new.
> 
> That argues for a better executive summary at the top that allows a reader
> to quickly determine if the content below is interesting in the current
> context.  I have no issue with adding content, just with its removal.

That's probably fair, but I think there are often short messages for compile
breakages are followups to bugs that don't really add value by being there,
and they aren't touching different code, they are a followup to the original
change.

> > Let's take a different example (and this is on HEAD, not even stable).
> > 
> > https://svnweb.freebsd.org/changeset/base/277458
> > <https://svnweb.freebsd.org/changeset/base/277458>
> > 
> > Can you figure out what that change does in a 2 or 3 sentence summary?
> > 
> > I think it has something to do with building could images, but I have no
> > idea
> I at least got that it was about “cloud", not “could”. :-)
> 
> > really.  Which could images does it support?  How is it different from what
> > was there before (was this the initial cloud image stuff, or did this just
> > add more, because I thought we shipped cloud images for 10.1 which
> > predates
> > this)?

Aside from the typo, it seems you don't have an answer to these questions
either? :)

> > I'm not trying to pick on Glen, but that log is basically a stream of
> > conciousness piece which might be great for psychoanalaysis, but it's not
> > very accessible to someone wanting to know what changed.
> 
> Apart from the need for a top-level summary, aren’t you really complaining
> here about the content of the original commit messages?  That’s a different
> problem, which I agree we have at times.

No, I've seen merges to stable that follow the same pattern.  This just
happened to be a merge from a projects branch (instead of from HEAD to stable)
that I remembered well enough to look for.  It's a general question of how
merge commits are logged though, whether from projects -> head or from
head -> stable.

Surely when I merge some feature from a p4 or git branch you don't want me to
dump the individual log messages (about 1/3 of which would be "Compile" since
I often run my editor on a separate machine from where I build/test the
changes) into the commit to head?

> > Here's another example:
> > 
> > https://svnweb.freebsd.org/base?view=revision&revision=189075
> > <https://svnweb.freebsd.org/base?view=revision&revision=189075>
> > 
> > Sadly, this was my first "big" merge with SVN (was not at all feasible
> > with
> > CVS) and I did not list all the revisions.  Suffice it to say there were
> > something like 50+, many of which were one-liner bug fix commit logs.  Had
> > I duplicated all the logs that commit message might have been hundreds of
> > lines long, and very hard for a user to figure out what had actually
> > changed and why it mattered.
> 
> A user who doesn’t know enough to understand the individual changes today,
> may need that information in the future.  For today, they just need enough
> information to quickly determine if, for their current purposes, the rest
> of the commit message can be ignored.

So I do think that is an argument for having a summary unless you mean that
the way to quickly determine if a commit can be ignored is to just ignore all
merges. :)

> > Here's a (shorter) and more recent example:
> > 
> > https://svnweb.freebsd.org/base?view=revision&revision=266339
> > <https://svnweb.freebsd.org/base?view=revision&revision=266339>
> > 
> > This merges 20 commits that all contribute to implementing a single
> > feature, and yet for someone reading stable/10 commits I believe it is
> > clear what this one feature is.
> 
> That’s great if the goal of reading the commit message is to find out if the
> feature was merged.  What if, as a side effect, the commit also touched
> another area - an area you are trying to debug?  It may be possible to
> determine that a related file was modified, but the rational for why it was
> modified is now gone.  That’s a shame.

I think if you are trimming the message, you don't trim notice of side
effects.  I do think you should still read the commit logs of all the changes
you are merging while composing the message.  In the last commit above, that
log message actually highlights some of the larger changes by pulling lines
from multiple of the listed commits into the summary.

It is true that this requires time, and if folks feel that is a waste of
time, so be it.  OTOH, if the issue is that you don't trust folks to edit
commit logs during a MFC, why do you trust them to write a commit log for a
commit to HEAD?

-- 
John Baldwin