[MPlayer-dev-eng] Re: PATCH [0/12] CoreAVC support (Take 3)

The Wanderer inverseparadox at comcast.net
Wed Feb 28 13:36:01 CET 2007


Alan Nisota wrote:

> Alex Beregszaszi wrote:
> 
>> Hi,

>>> 002dstfmt.patch: Ignore an error on connecting the output pin on
>>> filter
>> 
>> Maybe good, but remove the unneded cosmetics.
> 
> Not sure what cosmetics you mean.  The only 'cosmetics' are a 2 line
> comment explaining WTF is going on.

The cosmetics he's referring to are the reindentation of several lines -
most specifically the "if (result)" block, but possibly also the "
result = This->m_pOutputPin->vt->ReceiveConnection(This->m_pOutputPin,"
line in the first section. MPlayer policy dictates that such changes -
that is, speaking approximately, changes which do not modify the content
of a line as far as the compiler output is concerned - be made in
separate patches from any changes which have functional effect.

The reason for this policy is, so far as I can tell, purely to make
patches easier to read and thus to review (and to find bugs in, in the
event that a later binary regression search narrows the introduction of
a bug down to the specific patch).

-- 
       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 MPlayer-dev-eng mailing list