[MPlayer-cvslog] r27550 - in trunk: libmpdemux/demux_mkv.c mpcommon.c

[Diego Biurrun]

On Wed, Sep 10, 2008 at 08:57:13AM +0200, Reimar Doeffinger wrote:
> On Wed, Sep 10, 2008 at 01:17:06AM +0200, Diego Biurrun wrote:
> > On Tue, Sep 09, 2008 at 11:23:32PM +0200, Dominik 'Rathann' Mierzejewski wrote:
> > > On Tuesday, 09 September 2008 at 23:16, Diego Biurrun wrote:
> > > > On Tue, Sep 09, 2008 at 09:11:41AM +0200, Reimar Döffinger wrote:
> > > > > On Mon, Sep 08, 2008 at 11:26:22PM +0200, uau wrote:
> > > > > > 
> > > > > > Log:
> > > > > > Revert bad changes to SSA/ASS subtitle packet format
> > > > > > [...]
> > > > > 
> > > > > That is not an acceptable commit message. If it needs such a long
> > > > > explanation, explain it on the mailing list and reference the mail, but
> > > > > commit messages are supposed to be messages, not whole reference books.
> > > > 
> > > > I disagree.  It's better to have it in the log message than on the ml.
> > > 
> > > I agree with Reimar. Such long commit messages take a while to read.
> > > IMHO the point of a commit message is to describe the changes concisely
> > > in a couple of sentences so that one can look at "svn log" output without
> > > needing the diffs.
> > 
> > No.  The diffs can never explain why the change they describe was made,
> > they only show you what the change consists of.  Very often, it is not
> > at all easy to deduce one from the other.  This is an example of such a
> > case.
> 
> And the commit message does not help too much, because in the load of text it
> is very hard to find the relevant information.
> Stuff like "AFAIK" do only in very exceptional circumstances belong there,
> and I do not see that here.

I think it is useful, it means that new information will change the
situation.

> The text is also prose, instead of e.g. a list of abbreviated reasons.

This might indeed have helped.

> but only _in addition_ to some kind of abreviated "executive summary" of max. 4 lines
> that mentions the major what and whys.

I agree that there should be an executive summary, but this message does
actually have one:

  Revert bad changes to SSA/ASS subtitle packet format

  The following commits are reverted partially or completely:
  "a valid ASS line contains 9 ',' before actual text"
  "demux_mkv: output correctly formated ASS packets"
  "libass: add a new ass_process_data() to process demuxed subtitle packets"

Diego