[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