[MPlayer-cvslog] r25894 - trunk/libmpcodecs/native/alaw.h

[Ivan Kalvachev]

On Jan 27, 2008 11:36 PM, Diego Biurrun <diego at biurrun.de> wrote:
>
> On Sun, Jan 27, 2008 at 10:23:44PM +0100, Roberto Togni wrote:
> > On Sun, 27 Jan 2008 21:57:02 +0100
> > Diego Biurrun <diego at biurrun.de> wrote:
> >
> > > On Sun, Jan 27, 2008 at 08:32:56PM +0100, Roberto Togni wrote:
> > > > On Sun, 27 Jan 2008 19:49:25 +0100 (CET)
> > > > diego <subversion at mplayerhq.hu> wrote:
> > > >
> > > > > Log:
> > > > > Replace with the output of the updated alaw-gen generator program.
> > > > > This adds multiple inclusion guards and reformats the tables.
> > > >
> > > > WTF?
> > > >
> > > > There is no need to reformat the file to add multiple inclusion
> > > > protection.
> > > > And the table was already aligned.

The table was aligned, but there wasn't space after the comma
and this is major violation in the ministry of proper formating.

Rezistense is futile. You will be assimilated.

> > > >
> > > > Please revert.
> > >
> > > Umm, why?  Because the commit mixes multiple changes?  Look at the
> > > changes I did to alaw-gen.c beforehand.  This commit replaces the old
> > > header file with the output of the new alaw-gen program...
> >
> > Because you don't need to replace the file to add 3 lines, even if the
> > file was originally generated by a program; now that the file is in svn
> > it should be treated like other non-generated files (eg. the const was
> > added by hand). Replacing the file make sense only if you are changing
> > the content of all the tables for non-cosmetic reasons (eg. if they are
> > broken, or you need to change the values to float, ...)
> >
> > Because of the huge useless cosmetic changes, in violation of our
> > policy (you're not allowed to change formatting of files with no good
> > reason)
>
> This is different because this is a generated file and the commits to
> the generating program were properly split.

And you just committed the changes to the generating program, in order
to output useless cosmetic changes. Sorry but I'm feeling like this is
self induced justification.
Especially when the original change is not justified to begin with.

So you have to make 2 reverts.

> The point of this commit was not to add the multiple inclusion guards
> but to update alaw.h to conform to the output of the updated alaw-gen
> program.

And I guess you haven't read my explanation why including multiple
inclusion guards is not mplayer policy and why it is better not to do
it (at least in the way you do it now).
Yes I know, a lot of files have it, but they are the one that have to
be reworked.


> That Rich added the const to the file without updating the generator
> program was an oversight IMO.

I agree on that. That commit should stay.

> I'm still not sure if you want to see this commit split or reverted and
> not redone at all.

Nobody have asked you to redo it, yet.