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

[Clément Bœsch]

On Thu, May 10, 2012 at 12:24:42PM +0200, Michael Niedermayer wrote:
> On Wed, May 09, 2012 at 11:16:35PM +0200, Clément Bœsch wrote:
> > 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?
> 
> if my reply sounded like a objection please disregard it.
> 
> about the patchset in general, what i forgot is that i think
> checkheaders should be treated like warnings not errors if it is added
> to fate.
> 

Just to avoid any confusion, by adding it to FATE I meant just have a
dedicated instance running the checkheaders rule, not making the
checkheaders rule as part of make fate. So if something goes wrong, only
that FATE instance will be affected.

-- 
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/20120510/e409c773/attachment.asc>