[Ffmpeg-devel] Re: [PATCH] DVCPRO50 support

Michael Niedermayer michaelni
Tue Mar 7 09:36:52 CET 2006


Hi

On Tue, Mar 07, 2006 at 08:24:24AM +0200, Oded Shimon wrote:
> On Mon, Mar 06, 2006 at 10:02:25PM -0800, Roman Shaposhnik wrote:
> > On Mon, Mar 06, 2006 at 01:01:28PM +0100, Michael Niedermayer wrote:
> > > > Sorry about that... I wasn't sure whether or not to re-indent the big
> > > > blocks that got surrounded by new for() loops :). This should be the
> > > > last such change from me. Just set n_difchan=4 and you've got DV100
> > > > (well, almost :).
> > > 
> > > they should be reindented but in a seperate cosmetic only patch
> > > (for files maintained by me i would have rejected the cosmetic-functional
> > > mix)
> > 
> >   I'm just curious, would you actually approve a patch lacking proper 
> >   indentation ? Something that basically adds an outer if or for 
> >   statement around large existing chunks of code without reindenting
> >   them?

yes, if all the new code itself is indented correctly


> > 
> >   I mean, after all, things like:
> > 
> >   if (cond) {
> >   existing code
> >   which wasn't 
> >   reindented
> >   } else {
> >     small chunk
> >     of new properly
> >     indented code
> >   }
> > 
> >   look rather ugly.
> > 
> >   Again -- as a reviewer I was presented with a need to filter these
> >   indentation changes out to focus on the actual guts of the patch.
> 
> The intent is to have both cvslog and code readability by _seperating_ the 
> patches - one functional (which adds the if), and one cosmetic (which does 
> nothing but change spaces)

exactly

[...]
-- 
Michael





More information about the ffmpeg-devel mailing list