[FFmpeg-cvslog] r14267 - trunk/libavcodec/ra288.c

[Uoti Urpala]

On Fri, 2008-07-18 at 22:20 +0200, Michael Niedermayer wrote:
> On Fri, Jul 18, 2008 at 07:13:11PM +0300, Uoti Urpala wrote:
> > > As a sideeffect that would also work quite well with the existing messages
> > > while existing messages would not be compatible with "the kernel way"
> > 
> > You could create a script that would create acceptable messages for some
> > subset of commits. 
> 
> "acceptable" by what standards? by yours?

Any script is likely to either miss needed information or add
significant clutter to some messages.

> The extra module names are usefull in some cases and useless and clutter
> screen space in others.

Enough context information to enable parsing the commit message is
always needed when viewing project-wide logs so it is never clutter. And
usually it fits in small enough space that it does not add much extra
clutter even if you already know something about the commits you're
viewing such as that they all touch a particular file. What's currently
doable automatically if context information is missing, adding a full
list of changed files to each commit, creates a much higher amount of
clutter.

> > However making it work with all the various frontends
> > through which commit messages can be seen would be a big practical
> > problem
> 
> If you want to fix all bugs (not limited to the display of modules) in all
> frontends, feel free to do so. Failing that, the inability of one or
> more frontends to deal with something is of no relevance to me.
> We also wont split files in less than 100byte each if theres a buggy
> frontend around that segfaults otherwise. What matters are the tools
> actually used by the ffmpeg developers.

The tools currently used by FFmpeg developers do not support it, and
there are several such tools.

> Besides, how many frontends have the ability to remove module names
> when they are redundant and just clutter screen space? Are you going
> to fix them all before adding module names to your commit messages?

As explained above they normally take little space or time to read (much
less space and time to read than a separate file list).

> > so adding the context explicitly to all new messages would still
> > be preferable. 
> 
> preferable by you, but you are not a ffmpeg developer. I think mans
> is the only one that is really in favor of this and is a ffmpeg developer.
> I dont remember if diego just complained about the general low quality
> of commit messages or if he also wanted module: prefixes.

I don't care so much whether it's an explicit module prefix, but log
messages should make sense without reading the patch or file list first
and should give some rough idea about the change after reading just the
first line even if a full explanation takes longer.

> > There are also some smaller problems like text layout (if
> > you can't see how much space the automatic information part used when
> > writing the message).
> 
> theres various information like the commiter name, revission number, date
> number of lines changed ...
> combining these in a vissually clean way is the job of the frontend, an
> additional word for the module is not particularly hard to add in there.
> (as can be seen by my script)

All those others are optional information and have an obvious way to
show the value in the most efficient manner when they're needed. A
frontend cannot know what is the most efficient way to give enough
context to allow understanding a commit message that's missing needed
information. Your script didn't do a particularly good job in even the
simple cases. It produced for example
M format/rtpdec.c  M format/rtpenc.c
That already takes so much space that it's hard to meaningfully describe
the commit on one line.
The "RTP: " in the original gave enough information about the changed
files.

> Having it hardcoded in and not logically seperated from the commit message
> is more limiting, as it has to be removed and this only works if a strict
> format is followed by the developers.

Normally there's little need to remove such information as it doesn't
take much space.

> > > > If you want to create a script to start your editor with "ra288:"
> > > > already filled in when you start writing a commit message feel free to
> > > > do so. 
> > > 
> > > > But trying to automatically fill it in later with no human check
> > > > of the result doesn't work well.
> > > 
> > > No? Could you mail me your paper with the proofs and extensive tests,
> > > i seem to be unable to find it on citeseer.
> > 
> > This is again such a stupid "argument" that I wonder why you posted it
> > at all. Obviously nobody has written anything with "proofs and extensive
> > tests" (and a strict "proof" of something being impractical would be
> > hard in general).
> 
> Well you make claims as if everything was fact. While really you are just
> guessing wildly and leaning toward what best fits your current agenda.
> The english language has words for expressing ones lack of certainity and
> knowledge.

I don't feel a need to express uncertainty when I say that adding
missing information to log messages with your own custom scripts is not
a practical solution.

> Second, commit messages and other things should be done so that most
> developers are happy about how its done. Again we clearly have more
> people (reimar, vitor, me ...) that see no need to add module names
> brute force by hand everywhere. 

Reimar's commit messages at least usually (though not always) make sense
without reading the patch or file list first. Vitor's on the other hand
tell little if read separately.

> Third, I did post a one line script, showing how to add the filenames
> cleanly to the svn log output.

Which didn't give a particularly good result.

> You are welcome to write your own if the output displeases you. Or if you
> want information different from the file names that isnt in the commit
> message already.

I don't think that the job can be done well by a script. Most commits
can be reasonably understood by reading the patch but using commit
messages is more efficient. Similarly a file list can give missing
context but adding a couple of words to the commit message is more
efficient.