On Mon, Jun 06, 2011 at 05:52:16PM +0100, Mans Rullgard wrote:
> Signed-off-by: Mans Rullgard <[email protected]>

Hmm, well, you are fond of splitting Makefiles and everything
else is already split so I don't mind.

> --- a/Makefile
> +++ b/Makefile
> @@ -181,13 +181,7 @@ uninstall-data:
>  
> -clean:: testclean
> +clean::

What's the benefit of moving the dependency to the subdirectory Makefile?

> @@ -201,119 +195,7 @@ distclean::
>  
> -check: test checkheaders

I'm not sure that target should be moved to tests/, checkheaders is not
defined there.

> --- /dev/null
> +++ b/tests/Makefile
> @@ -0,0 +1,119 @@
> +include $(SRC_PATH_BARE)/tests/fate.mak
> +include $(SRC_PATH_BARE)/tests/fate2.mak
> +
> +include $(SRC_PATH_BARE)/tests/fate/aac.mak
> +include $(SRC_PATH_BARE)/tests/fate/als.mak
> +include $(SRC_PATH_BARE)/tests/fate/fft.mak
> +include $(SRC_PATH_BARE)/tests/fate/h264.mak
> +include $(SRC_PATH_BARE)/tests/fate/mp3.mak
> +include $(SRC_PATH_BARE)/tests/fate/vorbis.mak
> +include $(SRC_PATH_BARE)/tests/fate/vp8.mak

unrelated: 

  include $(SRC_PATH_BARE)/tests/fate/*.mak

> +FATE = $(FATE_ACODEC)                                                   \
> +       $(FATE_VCODEC)                                                   \
> +       $(FATE_LAVF)                                                     \
> +       $(FATE_LAVFI)                                                    \
> +       $(FATE_SEEK)                                                     \

extra karma: Maybe you can cut down on that silly amount of whitespace
while moving the line anyway.

> +ifdef SAMPLES
> +FATE += $(FATE_TESTS)

unrelated: This reminds me that I'd prefer for SAMPLES to be renamed to
FATE_SAMPLES, just as the environment variable.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to