[FFmpeg-devel] DVCPRO HD: request for review

The Wanderer inverseparadox
Sat Aug 30 22:36:45 CEST 2008

Roman V. Shaposhnik wrote:

> On Sat, 2008-08-30 at 00:13 +0200, Michael Niedermayer wrote:

>>> And I missed it deliberately. I can see quite clearly why
>>> submitting patches generated with the moral equivalent of "diff
>>> -w" is a very desirable thing to do. However, as a matter of
>>> established *commit* practice I don't see any value in doing a
>>> commit that butchers established indentation rules be followed by
>>> the commit that restores them. Please explain.
>> As you say you see the sense in it for a patch, why would that
>> sense not apply to svn?
> Because -w is for human consumption and commits are machine
> transactions.

But commit logs are for human consumption, and need to be readable for
largely the same reasons that submitted patches do.

> And as far as transactions go, when I add an else clause to the
> existing if statement the transaction has both indentation of the old
> code and addition of the new one.

I'll grant that on the "each commit should be a self-contained
transaction" idea, cosmetic changes should be part of the commit; it's
conceptually clean and makes logical sense, and as such I would in
principle be inclined to argue in favor of it. In practice, however, I
think that the value of that particular form of Doing It Right is
outweighed by the difficulties it introduces.

> Now, if you're worried about "software archeology" you can always use
> diff -w quite easily and have the benefit of both worlds. Well, at
> least with the reasonable SCMs you can.

But not with just following the commit-log mailing list, which is the
primary way (or one of the primary ways) past changes are tracked and
investigated around here.

>> a odd bug caused by a 170k change is a lot harder to debug with a
>> binary search in the history than one caused by a 5k change, simply
>> because there is less code that can have caused the bug.
> We are not talking about 170k of a diff here. The above statement is
> for a specific case of adding an else clause.

I've seen some pretty big if/else statements...

       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

Secrecy is the beginning of tyranny.

More information about the ffmpeg-devel mailing list