[MPlayer-cvslog] CVS: main configure, 1.1044, 1.1045 Makefile, 1.329, 1.330

Ivan Kalvachev ikalvachev at gmail.com
Sun Aug 21 14:00:13 CEST 2005


2005/8/21, Michael Niedermayer <michaelni at gmx.at>:
> Hi
> 
> On Sun, Aug 21, 2005 at 06:25:58AM +0200, Diego Biurrun wrote:
> > On Sat, Aug 20, 2005 at 09:30:04PM -0400, Rich Felker wrote:
> > > On Sat, Aug 20, 2005 at 06:46:36PM +0200, Diego Biurrun wrote:
> > > >
> > > > BTW, can we all try to keep cvs log messages a bit more descriptive?
> > > > Things like 10l may be fun and quick to type and the problem may even be
> > > > obvious to those in the know, but don't forget that many people are
> > > > around to learn and explanations help them.
> > >
> > > Are you talking about my v4l2 commit? IMO the error is obvious from
> > > the (one-line) patch. Dividing int/int to get a float does not work.
> > > Normally I say a lot more than just "10l" (or whatever amount) except
> > > when the error is obvious to anyone who knows C, or a typo, or a wrong
> > > numbers, etc.
> >
> > Let's say that it was inspired by that commit, but I'm talking in
> > general, there are many more examples.  Yes, it will be obvious to those
> > in the know, but if you look at the output of 'cvs log' then 10l is not
> > descriptive unless you see the diff as well, while something like
> > "int/int does not produce a float value" is...
> 
> this is still not good, the commit log should at least contain
> * high level description of what the change does (fixing fps calculation
>   in setup_audio_buffer_sizes()) but actually thats still not good as it
>   says nothing about the conequences of the wrong fps ...
> * low level descrioption / diff summary like (int/int does not produce...)
> * list of bug numbers / mailing list archive links which get fixed
> * possible unwanted sideeffects of the change
> * summary of any test/benchmarks done
> * severity of the bug/change
> * some list of tags from (cosmetic/crash-fix/rounding-fix/security-fix/
>   spelling-fix/indention-fix/untested/new-feature/optimization)

Somehow I cannot imagine that You Michael will write an cvs explanation
6 times bigger than the patch itself ;) 
After all you are the master of 10^x,l cola explanations ;)

Anyway seriously, the text should say what it fix, the bug number etc..

Severity is a litlle overkill,and benchmarks too except when the
change is in time critical section (e.g. inside codec loop), even then
it should say faster/slower and probable %.
If there are sideeffects then the fix is bad.
This applies to the done test too, I mean it is developer jobs to test
the commit and finds it OK, aka no easy to detect side effects. There
is no need to write what test have been done (should we write the test
case, test programs, test results, just to show it is ok).

It is also not good idea to mark security-fix, as there are noobies
that cann't find them without these tags :)




More information about the MPlayer-cvslog mailing list