[FFmpeg-devel] [PATCH 11/17] lavc/dcaenc: fix make checkheaders.

[Clément Bœsch]

On Wed, May 09, 2012 at 03:33:10PM +0200, Michael Niedermayer wrote:
> On Wed, May 09, 2012 at 01:25:39PM +0200, Clément Bœsch wrote:
> > On Wed, May 09, 2012 at 12:45:07PM +0200, Michael Niedermayer wrote:
> > > On Wed, May 09, 2012 at 10:01:36AM +0200, Clément Bœsch wrote:
> > > > ---
> > > >  libavcodec/dcaenc.h |    2 ++
> > > >  1 file changed, 2 insertions(+)
> > > 
> > > does it make sense to run check headers on non public headers ?
> > > 
> > 
> > It's certainly simpler to check them all. Do you think all these checks
> > are a problem?
> 
> I think the problem lies with how the checks are seen.
> What we need to do is fix bugs, improve maintainability and readability
> the checks can point to problems, these problems should be fixed.
> but when the check points to correct code care must be taken not to
> worsen the code for no other point than to hide the warning.
> Here the check should be fixed if no trivial change to the code can
> solve it.
> 
> For example i dont think moving an include from the top of the file
> to the middle is really a good idea for maintainability.

I agree, and sent a new version of the lavu/error.h patch.

> Also i think os2threads should fail on non os2 systems, it could be
> added to SKIPHEADERS if the OS is not OS2. adding ifdefs into it
> so it can be included but does nothing at all on other platforms
> doesnt seem like a good idea to me

Agree too, and new version sent for this one too.

About the dcaenc.h patch, is there something wrong with it?

[..]


-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120509/ee6fcbdc/attachment.asc>