[FFmpeg-devel] [Fwd: Summer of code small task patch]

[Michael Niedermayer]

On Sat, Mar 28, 2009 at 01:52:50PM +0100, Diego Biurrun wrote:
> On Sat, Mar 28, 2009 at 01:30:39PM +0100, Michael Niedermayer wrote:
> > On Sat, Mar 28, 2009 at 12:42:46PM +0100, Diego Biurrun wrote:
> > > On Sat, Mar 28, 2009 at 06:23:14AM +0100, Michael Niedermayer wrote:
> > > > On Fri, Mar 27, 2009 at 10:18:07PM +0200, Dylan Yudaken wrote:
> > > > >
> > > > > --- libavcodec/dct-test.c	(revision 18203)
> > > > > +++ libavcodec/dct-test.c	(working copy)
> > > > > @@ -46,9 +46,9 @@
> > > > >  void *fast_memcpy(void *a, const void *b, size_t c){return memcpy(a,b,c);};
> > > > >  
> > > > >  /* reference fdct/idct */
> > > > > -void fdct(DCTELEM *block);
> > > > > -void idct(DCTELEM *block);
> > > > > -void init_fdct(void);
> > > > > +void ff_ref_fdct(DCTELEM *block);
> > > > > +void ff_ref_idct(DCTELEM *block);
> > > > > +void ff_ref_dct_init(void);
> > > > 
> > > > renaming things should be a seperate patch
> > > > actually a patch should either do functional changes or non functional not
> > > > both
> > > 
> > > I think this should be treated differently, he is creating an entirely
> > > new and independent file.  Adding a renaming step in between is
> > > pointless extra work.
> > 
> > What do you suggest exactly?
> 
> Add the new file and apply the renamings in one go.

Patches should not mix unrelated changes, and even more so for a
qualification task.


>  I see no benefit in
> splitting this.

This though does not mean others dont.


> The renamings are straightforward and I verified the
> patch to be correct.

you verified that the renamings are just renamings?
or that the whole patch is correct?


>  It's thus fully reviewed and trivial, I'd like to
> apply the patch as-is when the other issues you pointed out have been
> addressed.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090328/cfa92774/attachment.pgp>