[FFmpeg-cvslog] [propchange]: r14167 - svn:log

Måns Rullgård mans
Sat Jul 12 19:11:58 CEST 2008


Michael Niedermayer <michaelni at gmx.at> writes:

> On Sat, Jul 12, 2008 at 02:38:43PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Fri, Jul 11, 2008 at 10:48:07PM +0200, Reimar D?ffinger wrote:
>> >> On Fri, Jul 11, 2008 at 09:37:37PM +0200, Michael Niedermayer wrote:
>> >> > On Fri, Jul 11, 2008 at 07:53:50PM +0100, M?ns Rullg?rd wrote:
>> >> > > michael <subversion at mplayerhq.hu> writes:
>> >> > > 
>> >> > > > Author: michael
>> >> > > > Revision: 14167
>> >> > > > Property Name: svn:log
>> >> > > > Action: modified
>> >> > > >
>> >> > > > Property diff:
>> >> > > > --- old property value
>> >> > > > +++ new property value
>> >> > > > @@ -1 +1,2 @@
>> >> > > >  slightly better detection
>> >> > > > +fixes misdetection of MPEG-PS (AVSEQ03.DAT and AVSEQ06.DAT)
>> >> > > 
>> >> > > Slightly better message.  I'd still like to see a mention of psxstr in
>> >> > > the message.
>> >> > 
>> >> > Then you will have to add it in your personal fork
>> >> > or start a vote on ffmpeg-dev and get a majority to support that.
>> >> 
>> >> I'd really prefer a bit less harsh words in that, it serves no purpose
>> >> (at least yet) (note this is directed to all...).
>> >
>> > Yes, and i had edited my reply to mans already twice to make it more
>> > diplomatic before sending.
>> > Iam not in particularly diplomatic mood though ...
>> 
>> Don't worry.  I can take the heat.
>> 
>> >> While I agree that it is possible to have the filenames displayed with
>> >> SVN/git and filter the log messages by filename, too, I also think it is
>> >> a pain. And while tend to include file names, I agree with Michael that
>> >> this is something that would be better fixed in tools.
>> >> But nevertheless, coming to this particular case: wouldn't you agree
>> >> that it would be to have a log message that really explains which
>> >> problem the patch fixes without looking at files lists or diffs?
>> >> IMO the following message would have done it:
>> >> "Fix MPEG-PS (AVSEQ03.DAT and AVSEQ06.DAT) misdetected as psxstr"
>> >> while the current message IMHO tells only have the story (esp. since
>> >> MPEG stuff is misdetected "all the time").
>> >
>> > I do not object to you or mans adding a "as psxstr" in this specific
>> > case. OTOH I very strongly object against adding the filename in general.
>> 
>> I never said the filename should be added.  I said module, or
>> functional unit.  This is something else.  In many cases, it coincides
>> with a filename, but in many others it does not.  Collections like
>> utils.c or dsputils.c are examples when the filename is mostly useless
>> information.  A simple mention of which codec or format is affected
>> (when the change is isolated), is all I'm asking for.
>> 
>> A few examples of bad messages:
>> 
>> - "typo", "typos", 53 commits
>> - "simplify" etc", 127 commits  simpler
>> - "optimize" etc, 18 commits
>> - "memleak", 3 commits
>> - "10l" etc, 107 commits
>> - "factorize", 5 commits
>> - "oops", 4 commits
>> - "align", "alignment", 3 commits
>> - "update", 34 commits
>> - "clean", "cleanup", 41 commits
>> - "forgot", "forgotten", 3 commits
>> - "bugfix", 3 commits
>> - "bugs" Did someone add bugs?
>> - "correction"
>> - "antitime" WTF does that mean?
>> - "100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l100l"
>> 
>> Overall, stupid single-word comments like these account for roughly
>> one in 15 commits.  This makes one-line logs much less useful than
>> they could be.
>
> I did never dispute that many commit messages are bad. Though i do not
> agree that all listed above are neccessarily bad.
> "factorize" or "simplify" are IMO fine. Also "typo" does not seem wrong
> one surely can be more verbose and say "Fixing a typo in a comment" or
> "Fixing a typo in a local variable name"
> But then i do not agree that "mpeg-ps: typo" is better than
> "Fixing a typo in a comment"

IMHO, the comment doesn't even need to mention the fact that the error
probably was a typo.  The source of the error is largely irrelevant.
What matter is what has been changed, and the effect of the change.

Several people seem to have associated "typo" with comment fixes.
This is not the case.  Many of the commits carrying the "typo" tag
have significant changes to code.  See for example r10250 and r13448.

>> > I also will not play mans secretary for his latest fantasies.
>> > Log messages never included the file or module name, its just
>> > mans coming up with that "great" idea in the last days.
>> 
>> This is not something I've come up with recently.  It has been
>> bothering me to some degree for a long time.  However, it's mostly
>> since starting to use git and it's powerful search tools, that I've
>> really felt hindered by bad messages.
>
> Doesnt sound like gits search tools are that powerfull if it cant
> be limited to files or modules of interrest.
> And if it can, then i do not see where the problem is ...

How can I use filters if there is no data to apply them to?

-- 
M?ns Rullg?rd
mans at mansr.com




More information about the ffmpeg-cvslog mailing list